From c13a67106b11b569989bc48f90d18320f51673e6 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Thu, 27 Oct 2022 11:15:33 +0000
Subject: [PATCH] Check rights on DownloadExport

---
 arkindex/documents/api/export.py        |  2 ++
 arkindex/documents/tests/test_export.py | 46 ++++++++++++++++++++-----
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/arkindex/documents/api/export.py b/arkindex/documents/api/export.py
index 70ffe29536..ab0b27a449 100644
--- a/arkindex/documents/api/export.py
+++ b/arkindex/documents/api/export.py
@@ -58,6 +58,8 @@ class CorpusExportAPIView(CorpusACLMixin, ListCreateAPIView):
 class DownloadExport(RetrieveAPIView):
     """
     Download a corpus export.
+
+    Guest access is required on private corpora.
     """
     queryset = CorpusExport.objects.none()
     permission_classes = (IsVerified, )
diff --git a/arkindex/documents/tests/test_export.py b/arkindex/documents/tests/test_export.py
index e72a9405e7..539cea687b 100644
--- a/arkindex/documents/tests/test_export.py
+++ b/arkindex/documents/tests/test_export.py
@@ -156,8 +156,32 @@ class TestExport(FixtureAPITestCase):
         presigned_url_mock.return_value = 'http://somewhere'
         self.client.force_login(self.superuser)
         export = self.corpus.exports.create(user=self.user, state=CorpusExportState.Done)
-        response = self.client.get(reverse('api:download-export', kwargs={'pk': export.id}))
-        self.assertEqual(response.status_code, status.HTTP_302_FOUND)
+        with self.assertNumQueries(3):
+            response = self.client.get(reverse('api:download-export', kwargs={'pk': export.id}))
+            self.assertEqual(response.status_code, status.HTTP_302_FOUND)
+        self.assertEqual(response.headers['Location'], 'http://somewhere')
+        self.assertEqual(presigned_url_mock.call_args_list, [
+            call(
+                'get_object',
+                Params={
+                    'Bucket': 'export',
+                    'Key': str(export.id),
+                },
+            )
+        ])
+
+    @patch('arkindex.project.aws.s3.meta.client.generate_presigned_url')
+    def test_download_export_public_corpus(self, presigned_url_mock):
+        presigned_url_mock.return_value = 'http://somewhere'
+        self.client.force_login(self.user)
+        self.corpus.public = True
+        self.corpus.save()
+        self.corpus.memberships.filter(user=self.user).delete()
+        export = self.corpus.exports.create(user=self.superuser, state=CorpusExportState.Done)
+
+        with self.assertNumQueries(4):
+            response = self.client.get(reverse('api:download-export', kwargs={'pk': export.id}))
+            self.assertEqual(response.status_code, status.HTTP_302_FOUND)
         self.assertEqual(response.headers['Location'], 'http://somewhere')
 
     def test_download_export_requires_login(self):
@@ -170,8 +194,9 @@ class TestExport(FixtureAPITestCase):
         self.user.save()
         self.client.force_login(self.user)
         export = self.corpus.exports.create(user=self.user, state=CorpusExportState.Done)
-        response = self.client.get(reverse('api:download-export', kwargs={'pk': export.id}))
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        with self.assertNumQueries(2):
+            response = self.client.get(reverse('api:download-export', kwargs={'pk': export.id}))
+            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
     def test_download_export_requires_guest(self):
         self.user.rights.all().delete()
@@ -179,12 +204,15 @@ class TestExport(FixtureAPITestCase):
         self.corpus.save()
         self.client.force_login(self.user)
         export = self.corpus.exports.create(user=self.user, state=CorpusExportState.Done)
-        response = self.client.get(reverse('api:download-export', kwargs={'pk': export.id}))
-        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+        with self.assertNumQueries(5):
+            response = self.client.get(reverse('api:download-export', kwargs={'pk': export.id}))
+            self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     def test_download_export_not_done(self):
         self.client.force_login(self.superuser)
         for state in (CorpusExportState.Created, CorpusExportState.Running, CorpusExportState.Failed):
-            export = self.corpus.exports.create(user=self.user, state=state)
-            response = self.client.get(reverse('api:download-export', kwargs={'pk': export.id}))
-            self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+            with self.subTest(state=state):
+                export = self.corpus.exports.create(user=self.user, state=state)
+                with self.assertNumQueries(3):
+                    response = self.client.get(reverse('api:download-export', kwargs={'pk': export.id}))
+                    self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
-- 
GitLab