From f329512c14865703708e7e38cd1944ac8f20ffb7 Mon Sep 17 00:00:00 2001 From: "eric.quintero@trailofbits.com" Date: Tue, 2 Jun 2026 16:26:08 +0000 Subject: [PATCH] Hide unpublished releases from public downloads --- apps/downloads/api.py | 26 ++++++++++++++++++- apps/downloads/tests/test_views.py | 41 +++++++++++++++++++++++++++--- apps/downloads/views.py | 7 +++++ apps/pages/views.py | 5 +++- 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/apps/downloads/api.py b/apps/downloads/api.py index 4d8f3010c..5da20d82f 100644 --- a/apps/downloads/api.py +++ b/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.""" diff --git a/apps/downloads/tests/test_views.py b/apps/downloads/tests/test_views.py index 1a05dfd40..fc77e375c 100644 --- a/apps/downloads/tests/test_views.py +++ b/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,7 +387,8 @@ 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) @@ -373,6 +396,11 @@ def test_get_release_file(self): 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) diff --git a/apps/downloads/views.py b/apps/downloads/views.py index 01dd4a33d..ffa79e628 100644 --- a/apps/downloads/views.py +++ b/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: diff --git a/apps/pages/views.py b/apps/pages/views.py index 8a0bb97f0..eff37d436 100644 --- a/apps/pages/views.py +++ b/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: