From 23780b90a2a9070008b3c466ddab2e2d0186d09b Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 3 Oct 2024 15:58:39 +0200 Subject: [PATCH] Build: don't show listing or detail view if project is spam (#11633) * Build: don't show listing or detail view if project is spam Check for `is_show_dashboard_denied` when accessing build listing or detail pages and don't show the dashboard if the project is marked as spam. I tried to write some test cases for this, but it's hard since we don't have `readthedocs-ext` in here. I didn't find any test for similar cases either. We could probably mock most of that code, but I don't think it's worth. I tested this locally and it works as expected at least. Closes https://github.com/readthedocs/readthedocs-ext/issues/554 * Tests for dashboard views on spam projects --- readthedocs/builds/tests/test_views.py | 32 +++++++++++++++++++ readthedocs/builds/views.py | 19 +++++++++-- readthedocs/projects/views/base.py | 17 ++++++++-- .../rtd_tests/tests/test_project_views.py | 9 ++++++ 4 files changed, 72 insertions(+), 5 deletions(-) diff --git a/readthedocs/builds/tests/test_views.py b/readthedocs/builds/tests/test_views.py index 9698f7ae3b4..f1c64abde0a 100644 --- a/readthedocs/builds/tests/test_views.py +++ b/readthedocs/builds/tests/test_views.py @@ -105,3 +105,35 @@ def _get_project(self, owners, **kwargs): @override_settings(RTD_ALLOW_ORGANIZATIONS=True) class CancelBuildViewWithOrganizationsTests(CancelBuildViewTests): pass + + +class BuildViewsTests(TestCase): + def setUp(self): + self.user = get(User, username="test") + self.project = get(Project, users=[self.user]) + self.version = get(Version, project=self.project) + self.build = get( + Build, + project=self.project, + version=self.version, + task_id="1234", + state=BUILD_STATE_INSTALLING, + ) + + @mock.patch( + "readthedocs.builds.views.BuildList.is_show_dashboard_denied_wrapper", + mock.MagicMock(return_value=True), + ) + def test_builds_list_view_spam_project(self): + url = reverse("builds_project_list", args=[self.project.slug]) + response = self.client.get(url) + self.assertEqual(response.status_code, 410) + + @mock.patch( + "readthedocs.builds.views.BuildDetail.is_show_dashboard_denied_wrapper", + mock.MagicMock(return_value=True), + ) + def test_builds_detail_view_spam_project(self): + url = reverse("builds_detail", args=[self.project.slug, self.build.pk]) + response = self.client.get(url) + self.assertEqual(response.status_code, 410) diff --git a/readthedocs/builds/views.py b/readthedocs/builds/views.py index 139c6f3cac0..f88c3320d68 100644 --- a/readthedocs/builds/views.py +++ b/readthedocs/builds/views.py @@ -22,6 +22,7 @@ from readthedocs.core.utils import cancel_build, trigger_build from readthedocs.doc_builder.exceptions import BuildAppError from readthedocs.projects.models import Project +from readthedocs.projects.views.base import ProjectSpamMixin log = structlog.get_logger(__name__) @@ -124,9 +125,20 @@ def _get_versions(self, project): ) -class BuildList(FilterContextMixin, BuildBase, BuildTriggerMixin, ListView): +class BuildList( + FilterContextMixin, + ProjectSpamMixin, + BuildBase, + BuildTriggerMixin, + ListView, +): filterset_class = BuildListFilter + def get_project(self): + # Call ``.get_queryset()`` to get the current project from ``kwargs`` + self.get_queryset() + return self.project + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) @@ -154,9 +166,12 @@ def get_context_data(self, **kwargs): return context -class BuildDetail(BuildBase, DetailView): +class BuildDetail(BuildBase, ProjectSpamMixin, DetailView): pk_url_kwarg = "build_pk" + def get_project(self): + return self.get_object().project + @method_decorator(login_required) def post(self, request, project_slug, build_pk): project = get_object_or_404(Project, slug=project_slug) diff --git a/readthedocs/projects/views/base.py b/readthedocs/projects/views/base.py index f0a092cc08c..f0250df70c3 100644 --- a/readthedocs/projects/views/base.py +++ b/readthedocs/projects/views/base.py @@ -100,14 +100,25 @@ class ProjectSpamMixin: project's dashboard is denied. """ - def get(self, request, *args, **kwargs): + def is_show_dashboard_denied_wrapper(self): + """ + Determine if the project has reached dashboard denied treshold. + + This function is wrapped just for testing purposes, + so we are able to mock it from outside. + """ if "readthedocsext.spamfighting" in settings.INSTALLED_APPS: from readthedocsext.spamfighting.utils import ( # noqa is_show_dashboard_denied, ) if is_show_dashboard_denied(self.get_project()): - template_name = "errors/dashboard/spam.html" - return render(request, template_name=template_name, status=410) + return True + return False + + def get(self, request, *args, **kwargs): + if self.is_show_dashboard_denied_wrapper(): + template_name = "errors/dashboard/spam.html" + return render(request, template_name=template_name, status=410) return super().get(request, *args, **kwargs) diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index ab4b40c05f0..74690b39acb 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -372,6 +372,15 @@ def test_project_versions_only_shows_internal_versons(self): self.assertNotIn(self.external_version, response.context["active_versions"]) self.assertNotIn(self.external_version, response.context["inactive_versions"]) + @mock.patch( + "readthedocs.projects.views.base.ProjectSpamMixin.is_show_dashboard_denied_wrapper", + mock.MagicMock(return_value=True), + ) + def test_project_detail_view_spam_project(self): + url = reverse("projects_detail", args=[self.pip.slug]) + response = self.client.get(url) + self.assertEqual(response.status_code, 410) + @mock.patch("readthedocs.core.utils.trigger_build", mock.MagicMock()) class TestPrivateViews(TestCase):