API V3: Allow other users to see build notifications from public projects (#11449)

* API V3: Allow other users to see build notifications from public projects

Fixes #11333

* Fix tests

* Fix

* Update docstring

Author: stsewd

readthedocs/api/v3/permissions.py

@@ -63,13 +63,9 @@ def has_permission(self, request, view):
         if view.detail and view.action in ("list", "retrieve", "superproject"):
             # detail view is only allowed on list/retrieve actions (not
             # ``update`` or ``partial_update``).
-            if view.basename not in (
-                "projects-notifications",
-                "projects-builds-notifications",
-            ):
-                # We don't want to give detail access to resources'
-                # notifications to users that don't have access to those
-                # resources.
+            if view.basename != "projects-notifications":
+                # We don't want to give detail access to projects'
+                # notifications to users that don't have access to the project.
                 return True
 
         if view.basename.startswith("projects"):

readthedocs/api/v3/tests/test_builds.py

@@ -4,6 +4,7 @@
 from django.urls import reverse
 
 from readthedocs.builds.constants import EXTERNAL
+from readthedocs.projects.constants import PRIVATE, PUBLIC
 from readthedocs.subscriptions.constants import TYPE_CONCURRENT_BUILDS
 from readthedocs.subscriptions.products import RTDProductFeature
 
@@ -114,27 +115,96 @@ def test_external_version_projects_versions_builds_list_post(self):
         expected["version"]["urls"]["vcs"] = "https://github.com/rtfd/project/pull/v1.0"
         self.assertDictEqual(response_json, expected)
 
-    def test_projects_builds_notifications_list(self):
+    def test_projects_builds_notifications_list_anonymous_user(self):
         url = reverse(
             "projects-builds-notifications-list",
             kwargs={
                 "parent_lookup_project__slug": self.project.slug,
                 "parent_lookup_build__id": self.build.pk,
             },
         )
+        expected_response = self._get_response_dict(
+            "projects-builds-notifications-list"
+        )
 
         self.client.logout()
+
+        # Project and version are public.
         response = self.client.get(url)
-        self.assertEqual(response.status_code, 401)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
+
+        # Project is private, version is public.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PUBLIC
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
+        # Project and version are private.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
+        # Project is public, but version is private.
+        self.project.privacy_level = PUBLIC
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
+    def test_projects_builds_notifications_list(self):
+        url = reverse(
+            "projects-builds-notifications-list",
+            kwargs={
+                "parent_lookup_project__slug": self.project.slug,
+                "parent_lookup_build__id": self.build.pk,
+            },
+        )
+        expected_response = self._get_response_dict(
+            "projects-builds-notifications-list"
+        )
+
+        self.client.logout()
 
         self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
         response = self.client.get(url)
+
+        # Project and version are public.
         self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
 
-        self.assertDictEqual(
-            response.json(),
-            self._get_response_dict("projects-builds-notifications-list"),
-        )
+        # Project is private, version is public.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PUBLIC
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
+
+        # Project and version are private.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
+
+        # Project is public, but version is private.
+        self.project.privacy_level = PUBLIC
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
 
     def test_projects_builds_notifications_list_other_user(self):
         url = reverse(
@@ -144,10 +214,40 @@ def test_projects_builds_notifications_list_other_user(self):
                 "parent_lookup_build__id": self.build.pk,
             },
         )
-
+        expected_response = self._get_response_dict(
+            "projects-builds-notifications-list"
+        )
+        self.client.logout()
         self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.others_token.key}")
+
+        # Project and version are public.
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
+
+        # Project is private, version is public.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PUBLIC
+        self.version.save()
         response = self.client.get(url)
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 404)
+
+        # Project and version are private.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
+        # Project is public, but version is private.
+        self.project.privacy_level = PUBLIC
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
 
         # User can see their own notifications.
         url = reverse(
@@ -191,6 +291,49 @@ def test_projects_builds_notifications_list_post(self):
         # We don't allow POST on this endpoint
         self.assertEqual(response.status_code, 405)
 
+    def test_projects_builds_notifitications_detail_anonymous_user(self):
+        url = reverse(
+            "projects-builds-notifications-detail",
+            kwargs={
+                "parent_lookup_project__slug": self.project.slug,
+                "parent_lookup_build__id": self.build.pk,
+                "notification_pk": self.notification_build.pk,
+            },
+        )
+        expected_response = self._get_response_dict(
+            "projects-builds-notifications-detail"
+        )
+        self.client.logout()
+
+        # Project and version are public.
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
+
+        # Project is private, version is public.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PUBLIC
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
+        # Project and version are private.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
+        # Project is public, but version is private.
+        self.project.privacy_level = PUBLIC
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
     def test_projects_builds_notifitications_detail(self):
         url = reverse(
             "projects-builds-notifications-detail",
@@ -200,19 +343,44 @@ def test_projects_builds_notifitications_detail(self):
                 "notification_pk": self.notification_build.pk,
             },
         )
+        expected_response = self._get_response_dict(
+            "projects-builds-notifications-detail"
+        )
 
         self.client.logout()
+        self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
+
+        # Project and version are public.
         response = self.client.get(url)
-        self.assertEqual(response.status_code, 401)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
 
-        self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
+        # Project is private, version is public.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PUBLIC
+        self.version.save()
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
 
-        self.assertDictEqual(
-            response.json(),
-            self._get_response_dict("projects-builds-notifications-detail"),
-        )
+        # Project and version are private.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
+
+        # Project is public, but version is private.
+        self.project.privacy_level = PUBLIC
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
 
     def test_projects_builds_notifitications_detail_other_user(self):
         url = reverse(
@@ -223,10 +391,40 @@ def test_projects_builds_notifitications_detail_other_user(self):
                 "notification_pk": self.notification_build.pk,
             },
         )
-
+        expected_response = self._get_response_dict(
+            "projects-builds-notifications-detail"
+        )
+        self.client.logout()
         self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.others_token.key}")
+
+        # Project and version are public.
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        self.assertDictEqual(response.json(), expected_response)
+
+        # Project is private, version is public.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PUBLIC
+        self.version.save()
         response = self.client.get(url)
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 404)
+
+        # Project and version are private.
+        self.project.privacy_level = PRIVATE
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
+
+        # Project is public, but version is private.
+        self.project.privacy_level = PUBLIC
+        self.project.save()
+        self.version.privacy_level = PRIVATE
+        self.version.save()
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 404)
 
     def test_projects_builds_notifitications_detail_post(self):
         url = reverse(

readthedocs/api/v3/views.py

@@ -2,6 +2,7 @@
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
 from django.db.models import Exists, OuterRef
+from django.shortcuts import get_object_or_404
 from rest_flex_fields import is_expanded
 from rest_flex_fields.views import FlexFieldsMixin
 from rest_framework import status
@@ -23,6 +24,7 @@
 from rest_framework.viewsets import GenericViewSet, ModelViewSet, ReadOnlyModelViewSet
 from rest_framework_extensions.mixins import NestedViewSetMixin
 
+from readthedocs.api.v2.permissions import ReadOnlyPermission
 from readthedocs.builds.models import Build, Version
 from readthedocs.core.utils import trigger_build
 from readthedocs.core.utils.extend import SettingsOverrideObject
@@ -477,6 +479,24 @@ class NotificationsBuildViewSet(
     serializer_class = NotificationSerializer
     queryset = Notification.objects.all()
     filterset_class = NotificationFilter
+    # We need to show build notifications to anonymous users
+    # on public builds (the queryset will filter them out).
+    # We allow project admins to edit notifications.
+    permission_classes = [ReadOnlyPermission | IsProjectAdmin]
+
+    def _get_parent_build(self):
+        """
+        Overriden to filter by builds the current user has access to.
+
+        This includes public builds from other projects.
+        """
+        build_pk = self._get_parent_object_lookup(self.BUILD_LOOKUP_NAMES)
+        project_slug = self._get_parent_object_lookup(self.PROJECT_LOOKUP_NAMES)
+        return get_object_or_404(
+            Build.objects.api(user=self.request.user),
+            pk=build_pk,
+            project__slug=project_slug,
+        )
 
     def get_queryset(self):
         build = self._get_parent_build()