Fix attachments

misago/threads/api/postingendpoint/attachments.py

@@ -12,7 +12,7 @@ from . import PostingEndpoint, PostingMiddleware
 
 class AttachmentsMiddleware(PostingMiddleware):
     def use_this_middleware(self):
-        return bool(self.user.acl_cache['max_attachment_size'])
+        return bool(self.user_acl['max_attachment_size'])
 
     def get_serializer(self):
         return AttachmentsSerializer(
@@ -20,6 +20,7 @@ class AttachmentsMiddleware(PostingMiddleware):
             context={
                 'mode': self.mode,
                 'user': self.user,
+                'user_acl': self.user_acl,
                 'post': self.post,
             }
         )
@@ -41,7 +42,7 @@ class AttachmentsSerializer(serializers.Serializer):
         validate_attachments_count(ids)
 
         attachments = self.get_initial_attachments(
-            self.context['mode'], self.context['user'], self.context['post']
+            self.context['mode'], self.context['user_acl'], self.context['post']
         )
         new_attachments = self.get_new_attachments(self.context['user'], ids)
 
@@ -69,12 +70,12 @@ class AttachmentsSerializer(serializers.Serializer):
             self.final_attachments += new_attachments
             self.final_attachments.sort(key=lambda a: a.pk, reverse=True)
 
-    def get_initial_attachments(self, mode, user, post):
+    def get_initial_attachments(self, mode, user_acl, post):
         attachments = []
         if mode == PostingEndpoint.EDIT:
             queryset = post.attachment_set.select_related('filetype')
             attachments = list(queryset)
-            add_acl(user, attachments)
+            add_acl(user_acl, attachments)
         return attachments
 
     def get_new_attachments(self, user, ids):

misago/threads/tests/test_attachments_middleware.py

@@ -1,6 +1,7 @@
 from rest_framework import serializers
 
-from misago.acl.testutils import override_acl
+from misago.acl import useracl
+from misago.acl.test import patch_user_acl
 from misago.categories.models import Category
 from misago.conf import settings
 from misago.threads import testutils
@@ -15,6 +16,14 @@ class RequestMock(object):
     def __init__(self, data=None):
         self.data = data or {}
 
+cache_versions = {"acl": "abcdefgh"}
+
+
+def patch_attachments_acl(acl_patch=None):
+    acl_patch = acl_patch or {}
+    acl_patch.setdefault("max_attachment_size", 1024)
+    return patch_user_acl(acl_patch)
+
 
 class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
     def setUp(self):
@@ -26,12 +35,8 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
 
         self.post.update_fields = []
 
-        self.override_acl()
         self.filetype = AttachmentType.objects.order_by('id').last()
 
-    def override_acl(self, new_acl=None):
-        override_acl(self.user, new_acl or {'max_attachment_size': 1024})
-
     def mock_attachment(self, user=True, post=None):
         return Attachment.objects.create(
             secret=Attachment.generate_new_secret(),
@@ -46,35 +51,42 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
 
     def test_use_this_middleware(self):
         """use_this_middleware returns False if we can't upload attachments"""
-        middleware = AttachmentsMiddleware(user=self.user)
-
-        self.override_acl({'max_attachment_size': 0})
-
-        self.assertFalse(middleware.use_this_middleware())
+        with patch_user_acl({'max_attachment_size': 0}):
+            user_acl = useracl.get_user_acl(self.user, cache_versions)
+            middleware = AttachmentsMiddleware(user=self.user, user_acl=user_acl)
+            self.assertFalse(middleware.use_this_middleware())
 
-        self.override_acl({'max_attachment_size': 1024})
-
-        self.assertTrue(middleware.use_this_middleware())
+        with patch_user_acl({'max_attachment_size': 1024}):
+            user_acl = useracl.get_user_acl(self.user, cache_versions)
+            middleware = AttachmentsMiddleware(user=self.user, user_acl=user_acl)
+            self.assertTrue(middleware.use_this_middleware())
 
+    @patch_attachments_acl()
     def test_middleware_is_optional(self):
         """middleware is optional"""
         INPUTS = [{}, {'attachments': []}]
 
+        user_acl = useracl.get_user_acl(self.user, cache_versions)
+
         for test_input in INPUTS:
             middleware = AttachmentsMiddleware(
                 request=RequestMock(test_input),
                 mode=PostingEndpoint.START,
                 user=self.user,
+                user_acl=user_acl,
                 post=self.post,
             )
 
             serializer = middleware.get_serializer()
             self.assertTrue(serializer.is_valid())
 
+    @patch_attachments_acl()
     def test_middleware_validates_ids(self):
         """middleware validates attachments ids"""
         INPUTS = ['none', ['a', 'b', 123], range(settings.MISAGO_POST_ATTACHMENTS_LIMIT + 1)]
 
+        user_acl = useracl.get_user_acl(self.user, cache_versions)
+
         for test_input in INPUTS:
             middleware = AttachmentsMiddleware(
                 request=RequestMock({
@@ -82,18 +94,22 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
                 }),
                 mode=PostingEndpoint.START,
                 user=self.user,
+                user_acl=user_acl,
                 post=self.post,
             )
 
             serializer = middleware.get_serializer()
             self.assertFalse(serializer.is_valid(), "%r shouldn't validate" % test_input)
 
+    @patch_attachments_acl()
     def test_get_initial_attachments(self):
         """get_initial_attachments returns list of attachments already existing on post"""
+        user_acl = useracl.get_user_acl(self.user, cache_versions)
         middleware = AttachmentsMiddleware(
             request=RequestMock(),
             mode=PostingEndpoint.EDIT,
             user=self.user,
+            user_acl=user_acl,
             post=self.post,
         )
 
@@ -106,16 +122,19 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
 
         attachment = self.mock_attachment(post=self.post)
         attachments = serializer.get_initial_attachments(
-            middleware.mode, middleware.user, middleware.post
+            middleware.mode, middleware.user_acl, middleware.post
         )
         self.assertEqual(attachments, [attachment])
 
+    @patch_attachments_acl()
     def test_get_new_attachments(self):
         """get_initial_attachments returns list of attachments already existing on post"""
+        user_acl = useracl.get_user_acl(self.user, cache_versions)
         middleware = AttachmentsMiddleware(
             request=RequestMock(),
             mode=PostingEndpoint.EDIT,
             user=self.user,
+            user_acl=user_acl,
             post=self.post,
         )
 
@@ -133,27 +152,27 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
         attachments = serializer.get_new_attachments(middleware.user, [other_user_attachment.pk])
         self.assertEqual(attachments, [])
 
+    
+    @patch_attachments_acl({'can_delete_other_users_attachments': False})
     def test_cant_delete_attachment(self):
         """middleware validates if we have permission to delete other users attachments"""
-        self.override_acl({
-            'max_attachment_size': 1024,
-            'can_delete_other_users_attachments': False,
-        })
-
         attachment = self.mock_attachment(user=False, post=self.post)
         self.assertIsNone(attachment.uploader)
 
+        user_acl = useracl.get_user_acl(self.user, cache_versions)
         serializer = AttachmentsMiddleware(
             request=RequestMock({
                 'attachments': []
             }),
             mode=PostingEndpoint.EDIT,
             user=self.user,
+            user_acl=user_acl,
             post=self.post,
         ).get_serializer()
 
         self.assertFalse(serializer.is_valid())
 
+    @patch_attachments_acl()
     def test_add_attachments(self):
         """middleware adds attachments to post"""
         attachments = [
@@ -161,12 +180,14 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
             self.mock_attachment(),
         ]
 
+        user_acl = useracl.get_user_acl(self.user, cache_versions)
         middleware = AttachmentsMiddleware(
             request=RequestMock({
                 'attachments': [a.pk for a in attachments]
             }),
             mode=PostingEndpoint.EDIT,
             user=self.user,
+            user_acl=user_acl,
             post=self.post,
         )
 
@@ -182,6 +203,7 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
         self.assertEqual([a['filename'] for a in self.post.attachments_cache],
                          attachments_filenames)
 
+    @patch_attachments_acl()
     def test_remove_attachments(self):
         """middleware removes attachment from post and db"""
         attachments = [
@@ -189,12 +211,14 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
             self.mock_attachment(post=self.post),
         ]
 
+        user_acl = useracl.get_user_acl(self.user, cache_versions)
         middleware = AttachmentsMiddleware(
             request=RequestMock({
                 'attachments': [attachments[0].pk]
             }),
             mode=PostingEndpoint.EDIT,
             user=self.user,
+            user_acl=user_acl,
             post=self.post,
         )
 
@@ -212,6 +236,7 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
         self.assertEqual([a['filename'] for a in self.post.attachments_cache],
                          attachments_filenames)
 
+    @patch_attachments_acl()
     def test_steal_attachments(self):
         """middleware validates if attachments are already assigned to other posts"""
         other_post = testutils.reply_thread(self.thread)
@@ -221,12 +246,14 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
             self.mock_attachment(),
         ]
 
+        user_acl = useracl.get_user_acl(self.user, cache_versions)
         middleware = AttachmentsMiddleware(
             request=RequestMock({
                 'attachments': [attachments[0].pk, attachments[1].pk]
             }),
             mode=PostingEndpoint.EDIT,
             user=self.user,
+            user_acl=user_acl,
             post=self.post,
         )
 
@@ -241,6 +268,7 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
         self.assertEqual(Attachment.objects.get(pk=attachments[0].pk).post, other_post)
         self.assertEqual(Attachment.objects.get(pk=attachments[1].pk).post, self.post)
 
+    @patch_attachments_acl()
     def test_edit_attachments(self):
         """middleware removes and adds attachments to post"""
         attachments = [
@@ -249,12 +277,14 @@ class AttachmentsMiddlewareTests(AuthenticatedUserTestCase):
             self.mock_attachment(),
         ]
 
+        user_acl = useracl.get_user_acl(self.user, cache_versions)
         middleware = AttachmentsMiddleware(
             request=RequestMock({
                 'attachments': [attachments[0].pk, attachments[2].pk]
             }),
             mode=PostingEndpoint.EDIT,
             user=self.user,
+            user_acl=user_acl,
             post=self.post,
         )
 

misago/threads/tests/test_attachmentview.py

@@ -3,19 +3,25 @@ import os
 from django.urls import reverse
 
 from misago.acl.models import Role
-from misago.acl.testutils import override_acl
+from misago.acl.test import patch_user_acl
 from misago.categories.models import Category
 from misago.conf import settings
 from misago.threads import testutils
 from misago.threads.models import Attachment, AttachmentType
 from misago.users.testutils import AuthenticatedUserTestCase
 
-
 TESTFILES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'testfiles')
 TEST_DOCUMENT_PATH = os.path.join(TESTFILES_DIR, 'document.pdf')
 TEST_SMALLJPG_PATH = os.path.join(TESTFILES_DIR, 'small.jpg')
 
 
+def patch_attachments_acl(acl_patch=None):
+    acl_patch = acl_patch or {}
+    acl_patch.setdefault("max_attachment_size", 1024)
+    acl_patch.setdefault("can_download_other_users_attachments", True)
+    return patch_user_acl(acl_patch)
+
+
 class AttachmentViewTestCase(AuthenticatedUserTestCase):
     def setUp(self):
         super().setUp()
@@ -36,16 +42,6 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
             extensions='pdf',
         )
 
-        self.override_acl()
-
-    def override_acl(self, allow_download=True):
-        acl = self.user.acl_cache.copy()
-        acl.update({
-            'max_attachment_size': 1000,
-            'can_download_other_users_attachments': allow_download,
-        })
-        override_acl(self.user, acl)
-
     def upload_document(self, is_orphaned=False, by_other_user=False):
         with open(TEST_DOCUMENT_PATH, 'rb') as upload:
             response = self.client.post(
@@ -64,8 +60,6 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
             attachment.uploader = None
             attachment.save()
 
-        self.override_acl()
-
         return attachment
 
     def upload_image(self):
@@ -77,25 +71,25 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
             )
         self.assertEqual(response.status_code, 200)
 
-        attachment = Attachment.objects.order_by('id').last()
-
-        self.override_acl()
-
-        return attachment
+        return Attachment.objects.order_by('id').last()
 
+    @patch_attachments_acl()
     def assertIs404(self, response):
         self.assertEqual(response.status_code, 302)
         self.assertTrue(response['location'].endswith(settings.MISAGO_404_IMAGE))
 
+    @patch_attachments_acl()
     def assertIs403(self, response):
         self.assertEqual(response.status_code, 302)
         self.assertTrue(response['location'].endswith(settings.MISAGO_403_IMAGE))
 
+    @patch_attachments_acl()
     def assertSuccess(self, response):
         self.assertEqual(response.status_code, 302)
         self.assertFalse(response['location'].endswith(settings.MISAGO_404_IMAGE))
         self.assertFalse(response['location'].endswith(settings.MISAGO_403_IMAGE))
 
+    @patch_attachments_acl()
     def test_nonexistant_file(self):
         """user tries to retrieve nonexistant file"""
         response = self.client.get(
@@ -107,6 +101,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
 
         self.assertIs404(response)
 
+    @patch_attachments_acl()
     def test_invalid_secret(self):
         """user tries to retrieve existing file using invalid secret"""
         attachment = self.upload_document()
@@ -120,15 +115,15 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
 
         self.assertIs404(response)
 
+    @patch_attachments_acl({"can_download_other_users_attachments": False})
     def test_other_user_file_no_permission(self):
         """user tries to retrieve other user's file without perm"""
         attachment = self.upload_document(by_other_user=True)
 
-        self.override_acl(False)
-
         response = self.client.get(attachment.get_absolute_url())
         self.assertIs403(response)
 
+    @patch_attachments_acl({"can_download_other_users_attachments": False})
     def test_other_user_orphaned_file(self):
         """user tries to retrieve other user's orphaned file"""
         attachment = self.upload_document(is_orphaned=True, by_other_user=True)
@@ -139,6 +134,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         response = self.client.get(attachment.get_absolute_url() + '?shva=1')
         self.assertIs404(response)
 
+    @patch_attachments_acl()
     def test_document_thumbnail(self):
         """user tries to retrieve thumbnail from non-image attachment"""
         attachment = self.upload_document()
@@ -154,6 +150,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         )
         self.assertIs404(response)
 
+    @patch_attachments_acl()
     def test_no_role(self):
         """user tries to retrieve attachment without perm to its type"""
         attachment = self.upload_document()
@@ -164,6 +161,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         response = self.client.get(attachment.get_absolute_url())
         self.assertIs403(response)
 
+    @patch_attachments_acl()
     def test_type_disabled(self):
         """user tries to retrieve attachment the type disabled downloads"""
         attachment = self.upload_document()
@@ -174,6 +172,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         response = self.client.get(attachment.get_absolute_url())
         self.assertIs403(response)
 
+    @patch_attachments_acl()
     def test_locked_type(self):
         """user retrieves own locked file"""
         attachment = self.upload_document()
@@ -184,6 +183,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         response = self.client.get(attachment.get_absolute_url())
         self.assertSuccess(response)
 
+    @patch_attachments_acl()
     def test_own_file(self):
         """user retrieves own file"""
         attachment = self.upload_document()
@@ -191,6 +191,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         response = self.client.get(attachment.get_absolute_url())
         self.assertSuccess(response)
 
+    @patch_attachments_acl()
     def test_other_user_file(self):
         """user retrieves other user's file with perm"""
         attachment = self.upload_document(by_other_user=True)
@@ -198,6 +199,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         response = self.client.get(attachment.get_absolute_url())
         self.assertSuccess(response)
 
+    @patch_attachments_acl()
     def test_other_user_orphaned_file_is_staff(self):
         """user retrieves other user's orphaned file because he is staff"""
         attachment = self.upload_document(is_orphaned=True, by_other_user=True)
@@ -211,6 +213,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         response = self.client.get(attachment.get_absolute_url() + '?shva=1')
         self.assertSuccess(response)
 
+    @patch_attachments_acl()
     def test_orphaned_file_is_uploader(self):
         """user retrieves orphaned file because he is its uploader"""
         attachment = self.upload_document(is_orphaned=True)
@@ -221,6 +224,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         response = self.client.get(attachment.get_absolute_url() + '?shva=1')
         self.assertSuccess(response)
 
+    @patch_attachments_acl()
     def test_has_role(self):
         """user retrieves file he has roles to download"""
         attachment = self.upload_document()
@@ -231,6 +235,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         response = self.client.get(attachment.get_absolute_url() + '?shva=1')
         self.assertSuccess(response)
 
+    @patch_attachments_acl()
     def test_image(self):
         """user retrieves """
         attachment = self.upload_image()
@@ -238,6 +243,7 @@ class AttachmentViewTestCase(AuthenticatedUserTestCase):
         response = self.client.get(attachment.get_absolute_url() + '?shva=1')
         self.assertSuccess(response)
 
+    @patch_attachments_acl()
     def test_image_thumb(self):
         """user retrieves image's thumbnail"""
         attachment = self.upload_image()