Hide unpublished releases from public download paths

apps/downloads/api.py

@@ -11,7 +11,7 @@
 from apps.downloads.serializers import OSSerializer, ReleaseFileSerializer, ReleaseSerializer
 from apps.pages.api import PageResource
 from pydotorg.drf import BaseAPIViewSet, BaseFilterSet, IsStaffOrReadOnly
-from pydotorg.resources import GenericResource, OnlyPublishedAuthorization
+from pydotorg.resources import GenericResource, OnlyPublishedAuthorization, StaffAuthorization
 
 
 class OSResource(GenericResource):
@@ -77,6 +77,22 @@ class Meta(GenericResource.Meta):
         abstract = False
 
 
+class OnlyPublishedReleaseFileAuthorization(StaffAuthorization):
+    """Only staff users can see files attached to unpublished releases."""
+
+    def read_list(self, object_list, bundle):
+        """Filter to files for published releases for non-staff users."""
+        if not bundle.request.user.is_staff:
+            return object_list.filter(release__is_published=True)
+        return super().read_list(object_list, bundle)
+
+    def read_detail(self, object_list, bundle):
+        """Return True only if the related release is published for non-staff users."""
+        if not bundle.request.user.is_staff:
+            return bundle.obj.release.is_published
+        return super().read_detail(object_list, bundle)
+
+
 class ReleaseFileResource(GenericResource):
     """Tastypie resource for individual release files."""
 
@@ -88,6 +104,7 @@ class Meta(GenericResource.Meta):
 
         queryset = ReleaseFile.objects.all()
         resource_name = "downloads/release_file"
+        authorization = OnlyPublishedReleaseFileAuthorization()
         fields = [
             "name",
             "slug",
@@ -173,6 +190,13 @@ class ReleaseFileViewSet(viewsets.ModelViewSet):
     permission_classes = (IsStaffOrReadOnly,)
     filterset_class = ReleaseFileFilter
 
+    def get_queryset(self):
+        """Return all files for staff, files for published releases for everyone else."""
+        queryset = super().get_queryset()
+        if self.request.user.is_staff:
+            return queryset
+        return queryset.filter(release__is_published=True)
+
     @action(detail=False, methods=["delete"])
     def delete_by_release(self, request):
         """Delete all release files associated with a given release."""

apps/downloads/tests/test_views.py

@@ -45,6 +45,12 @@ def test_download_release_detail(self):
         response = self.client.get(url)
         self.assertEqual(response.status_code, 404)
 
+    def test_download_release_detail_hides_unpublished_release(self):
+        url = reverse("download:download_release_detail", kwargs={"release_slug": self.draft_release.slug})
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+        self.assertNotIn(self.draft_release_linux.name, response.content.decode())
+
     def test_download_release_detail_not_superseded(self):
         """Test that latest releases and Python 2 do not show a superseded notice."""
         for release in [self.python_3, self.python_3_8_20, self.release_275]:
@@ -150,6 +156,22 @@ def test_redirect_page_object_to_release_detail_page(self):
             status_code=301,
         )
 
+    def test_redirect_page_object_to_release_detail_page_hides_unpublished_release(self):
+        legacy_page = PageFactory(
+            title="Python 9.7.2 Release",
+            path="download/releases/9.7.2",
+            content="legacy draft release page",
+            is_published=True,
+        )
+        self.draft_release.release_page = None
+        self.draft_release.save()
+
+        response = self.client.get(legacy_page.get_absolute_url())
+
+        self.assertEqual(response.status_code, 200)
+        self.assertNotIn(self.draft_release.get_absolute_url(), response.headers.get("Location", ""))
+        self.assertNotIn(self.draft_release_linux.name, response.content.decode())
+
 
 class RegressionTests(DownloadMixin, TestCase):
     """These tests are for bugs found by Sentry."""
@@ -365,14 +387,20 @@ def test_get_release_file(self):
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
         content = self.get_json(response)
-        self.assertEqual(len(content), 5)
+        self.assertEqual(len(content), 4)
+        self.assertNotIn(self.draft_release_linux.name, {release_file["name"] for release_file in content})
 
         url = self.create_url("release_file", self.release_275_linux.pk)
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
         content = self.get_json(response)
         self.assertEqual(content["name"], self.release_275_linux.name)
 
+        url = self.create_url("release_file", self.draft_release_linux.pk)
+        response = self.client.get(url)
+        # TODO: API v1 returns 401; and API v2 returns 404.
+        self.assertIn(response.status_code, [401, 404])
+
         url = self.create_url("release_file", 9999999)
         response = self.client.get(url)
         self.assertEqual(response.status_code, 404)
@@ -466,12 +494,19 @@ def test_filter_release_file(self):
         content = self.get_json(response)
         self.assertEqual(len(content), 1)
 
-        # Files for a draft release should be shown to users.
-        # TODO: We may deprecate this behavior when we drop API v1.
+        # Anonymous users should only see files attached to published releases.
         response = self.client.get(self.create_url("release_file", filters={"release": self.draft_release.pk}))
         self.assertFalse(self.draft_release.is_published)
         self.assertEqual(response.status_code, 200)
         content = self.get_json(response)
+        self.assertEqual(len(content), 0)
+
+        response = self.client.get(
+            self.create_url("release_file", filters={"release": self.draft_release.pk}),
+            headers={"authorization": self.Authorization},
+        )
+        self.assertEqual(response.status_code, 200)
+        content = self.get_json(response)
         self.assertEqual(len(content), 1)
 
 

apps/downloads/views.py

@@ -202,6 +202,13 @@ class DownloadReleaseDetail(DownloadBase, DetailView):
     model = Release
     context_object_name = "release"
 
+    def get_queryset(self):
+        """Return all releases for staff, published releases for everyone else."""
+        queryset = super().get_queryset()
+        if self.request.user.is_staff:
+            return queryset
+        return queryset.filter(is_published=True)
+
     def get_object(self):
         """Retrieve the release by slug or raise 404."""
         try:

apps/pages/views.py

@@ -58,8 +58,11 @@ def get(self, request, *args, **kwargs):
         matched = re.match(r"/download/releases/([\d.]+)/$", self.request.path)
         if matched is not None:
             release_slug = "python-{}".format(matched.group(1).replace(".", ""))
+            release_queryset = Release.objects.filter(slug=release_slug, release_page__isnull=True)
+            if not request.user.is_staff:
+                release_queryset = release_queryset.filter(is_published=True)
             try:
-                Release.objects.get(slug=release_slug, release_page__isnull=True)
+                release_queryset.get()
             except Release.DoesNotExist:
                 pass
             else: