Merge branch 'bugfix' into 'master'

Fix huge permission problem inducing server memory leaks for etickets

See merge request ae/Sith!251

club/tests.py

@@ -641,7 +641,7 @@ class MailingFormTest(TestCase):
             {"action": MailingForm.ACTION_NEW_MAILING, "mailing_email": "mde"},
         )
         mde = Mailing.objects.get(email="mde")
-        response = self.client.post(
+        self.client.post(
             reverse("club:mailing", kwargs={"club_id": self.bdf.id}),
             {
                 "action": MailingForm.ACTION_NEW_SUBSCRIPTION,
@@ -650,6 +650,11 @@ class MailingFormTest(TestCase):
                 "subscription_mailing": mde.id,
             },
         )
+
+        response = self.client.get(
+            reverse("club:mailing", kwargs={"club_id": self.bdf.id})
+        )
+
         self.assertContains(response, "comunity@git.an")
         self.assertContains(response, "richard@git.an")
         self.assertContains(response, "krophil@git.an")

club/views.py

@@ -574,7 +574,8 @@ class ClubMailingView(ClubTabsMixin, CanEditMixin, DetailFormView):
             except ValidationError as validation_error:
                 return validation_error
 
-            users_to_save.append(sub.save())
+            sub.save()
+            users_to_save.append(sub)
 
         if cleaned_data["subscription_email"]:
             sub = MailingSubscription(

core/views/__init__.py

@@ -140,6 +140,52 @@ def can_view(obj, user):
     return can_edit(obj, user)
 
 
+class GenericContentPermissionMixinBuilder(View):
+    """
+    Used to build permission mixins
+    This view protect any child view that would be showing an object that is restricted based
+      on two properties
+
+    :prop permission_function: function to test permission with, takes an object and an user an return a bool
+    :prop raised_error: permission to be raised
+
+    :raises: raised_error
+    """
+
+    permission_function = lambda obj, user: False
+    raised_error = PermissionDenied
+
+    @classmethod
+    def get_permission_function(cls, obj, user):
+        return cls.permission_function(obj, user)
+
+    def dispatch(self, request, *arg, **kwargs):
+
+        if hasattr(self, "get_object") and callable(self.get_object):
+            self.object = self.get_object()
+            if not self.get_permission_function(self.object, request.user):
+                raise self.raised_error
+            return super(GenericContentPermissionMixinBuilder, self).dispatch(
+                request, *arg, **kwargs
+            )
+
+        # If we get here, it's a ListView
+
+        queryset = self.get_queryset()
+        l_id = [o.id for o in queryset if self.get_permission_function(o, request.user)]
+        if not l_id and queryset.count() != 0:
+            raise self.raised_error
+        self._get_queryset = self.get_queryset
+
+        def get_qs(self2):
+            return self2._get_queryset().filter(id__in=l_id)
+
+        self.get_queryset = types.MethodType(get_qs, self)
+        return super(GenericContentPermissionMixinBuilder, self).dispatch(
+            request, *arg, **kwargs
+        )
+
+
 class CanCreateMixin(View):
     """
     This view is made to protect any child view that would create an object, and thus, that can not be protected by any
@@ -161,7 +207,7 @@ class CanCreateMixin(View):
         raise PermissionDenied
 
 
-class CanEditPropMixin(View):
+class CanEditPropMixin(GenericContentPermissionMixinBuilder):
     """
     This view is made to protect any child view that would be showing some properties of an object that are restricted
     to only the owner group of the given object.
@@ -171,28 +217,10 @@ class CanEditPropMixin(View):
     :raises: PermissionDenied
     """
 
-    def dispatch(self, request, *arg, **kwargs):
+    permission_function = can_edit_prop
-        try:
-            self.object = self.get_object()
-            if can_edit_prop(self.object, request.user):
-                return super(CanEditPropMixin, self).dispatch(request, *arg, **kwargs)
-            return forbidden(request)
-        except:
-            pass
-        # If we get here, it's a ListView
-        l_id = [o.id for o in self.get_queryset() if can_edit_prop(o, request.user)]
-        if not l_id and self.get_queryset().count() != 0:
-            raise PermissionDenied
-        self._get_queryset = self.get_queryset
-
-        def get_qs(self2):
-            return self2._get_queryset().filter(id__in=l_id)
-
-        self.get_queryset = types.MethodType(get_qs, self)
-        return super(CanEditPropMixin, self).dispatch(request, *arg, **kwargs)
 
 
-class CanEditMixin(View):
+class CanEditMixin(GenericContentPermissionMixinBuilder):
     """
     This view makes exactly the same thing as its direct parent, but checks the group on the edit_groups field of the
     object
@@ -200,28 +228,10 @@ class CanEditMixin(View):
     :raises: PermissionDenied
     """
 
-    def dispatch(self, request, *arg, **kwargs):
+    permission_function = can_edit
-        try:
-            self.object = self.get_object()
-            if can_edit(self.object, request.user):
-                return super(CanEditMixin, self).dispatch(request, *arg, **kwargs)
-            return forbidden(request)
-        except:
-            pass
-        # If we get here, it's a ListView
-        l_id = [o.id for o in self.get_queryset() if can_edit(o, request.user)]
-        if not l_id and self.get_queryset().count() != 0:
-            raise PermissionDenied
-        self._get_queryset = self.get_queryset
-
-        def get_qs(self2):
-            return self2._get_queryset().filter(id__in=l_id)
-
-        self.get_queryset = types.MethodType(get_qs, self)
-        return super(CanEditMixin, self).dispatch(request, *arg, **kwargs)
 
 
-class CanViewMixin(View):
+class CanViewMixin(GenericContentPermissionMixinBuilder):
     """
     This view still makes exactly the same thing as its direct parent, but checks the group on the view_groups field of
     the object
@@ -229,28 +239,7 @@ class CanViewMixin(View):
     :raises: PermissionDenied
     """
 
-    def dispatch(self, request, *arg, **kwargs):
+    permission_function = can_view
-
-        try:
-            self.object = self.get_object()
-            if can_view(self.object, request.user):
-                return super(CanViewMixin, self).dispatch(request, *arg, **kwargs)
-            return forbidden(request)
-        except:
-            pass
-        # If we get here, it's a ListView
-        queryset = self.get_queryset()
-
-        l_id = [o.id for o in queryset if can_view(o, request.user)]
-        if not l_id and queryset.count() != 0:
-            raise PermissionDenied
-        self._get_queryset = self.get_queryset
-
-        def get_qs(self2):
-            return self2._get_queryset().filter(id__in=l_id)
-
-        self.get_queryset = types.MethodType(get_qs, self)
-        return super(CanViewMixin, self).dispatch(request, *arg, **kwargs)
 
 
 class FormerSubscriberMixin(View):

counter/models.py

@@ -478,6 +478,10 @@ class Selling(models.Model):
         return user.is_owner(self.counter) and self.payment_method != "CARD"
 
     def can_be_viewed_by(self, user):
+        if (
+            not hasattr(self, "customer") or self.customer is None
+        ):  # Customer can be set to Null
+            return False
         return user == self.customer.user
 
     def delete(self, *args, **kwargs):

counter/views.py

@@ -1752,7 +1752,11 @@ class EticketPDFView(CanViewMixin, DetailView):
         from reportlab.graphics.barcode.qr import QrCodeWidget
         from reportlab.graphics import renderPDF
 
-        self.object = self.get_object()
+        if not (
+            hasattr(self.object, "product") and hasattr(self.object.product, "eticket")
+        ):
+            raise Http404
+
         eticket = self.object.product.eticket
         user = self.object.customer.user
         code = "%s %s %s %s" % (