From 75e155f4c0627daf3d83c9907549a27d37600da7 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 11 Jul 2024 17:21:08 -0500 Subject: [PATCH] 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 https://github.com/readthedocs/readthedocs.org/issues/11333 * Fix tests * Fix * Update docstring --- readthedocs/api/v3/permissions.py | 10 +- readthedocs/api/v3/tests/test_builds.py | 230 ++++++++++++++++++++++-- readthedocs/api/v3/views.py | 20 +++ 3 files changed, 237 insertions(+), 23 deletions(-) diff --git a/readthedocs/api/v3/permissions.py b/readthedocs/api/v3/permissions.py index 77d7b26afea..0c3765afe34 100644 --- a/readthedocs/api/v3/permissions.py +++ b/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"): diff --git a/readthedocs/api/v3/tests/test_builds.py b/readthedocs/api/v3/tests/test_builds.py index abd1b89349d..359fb1eb6d4 100644 --- a/readthedocs/api/v3/tests/test_builds.py +++ b/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,7 +115,7 @@ 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={ @@ -122,19 +123,88 @@ def test_projects_builds_notifications_list(self): "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( diff --git a/readthedocs/api/v3/views.py b/readthedocs/api/v3/views.py index 6cbdbc22722..3aa13905929 100644 --- a/readthedocs/api/v3/views.py +++ b/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()