Merge pull request #8792 from readthedocs/humitos/spam-deny-dashboard

Spam: deny dashboard on spammy projects

Author: humitos

pytest.ini

@@ -9,8 +9,6 @@ markers =
 python_files = tests.py test_*.py *_tests.py
 filterwarnings =
     # Ignore external dependencies warning deprecations
-    # textclassifier
-    ignore:The 'warn' method is deprecated, use 'warning' instead:DeprecationWarning
     # django-rest-framework
     ignore:Pagination may yield inconsistent results with an unordered object_list.*:django.core.paginator.UnorderedObjectListWarning
     # docutils

readthedocs/projects/exceptions.py

@@ -51,13 +51,3 @@ def get_default_message(self):
         if settings.ALLOW_PRIVATE_REPOS:
             return self.PRIVATE_ALLOWED
         return self.PRIVATE_NOT_ALLOWED
-
-
-class ProjectSpamError(Exception):
-
-    """
-    Error raised when a project field has detected spam.
-
-    This error is not raised to users, we use this for banning users in the
-    background.
-    """

readthedocs/projects/forms.py

@@ -11,15 +11,13 @@
 from django.contrib.auth.models import User
 from django.template.loader import render_to_string
 from django.utils.translation import ugettext_lazy as _
-from textclassifier.validators import ClassifierValidator
 
 from readthedocs.builds.constants import INTERNAL
 from readthedocs.core.history import SimpleHistoryModelForm
 from readthedocs.core.utils import slugify, trigger_build
 from readthedocs.core.utils.extend import SettingsOverrideObject
 from readthedocs.integrations.models import Integration
 from readthedocs.oauth.models import RemoteRepository
-from readthedocs.projects.exceptions import ProjectSpamError
 from readthedocs.projects.models import (
     Domain,
     EmailHook,
@@ -173,7 +171,6 @@ class Meta:
         )
 
     description = forms.CharField(
-        validators=[ClassifierValidator(raises=ProjectSpamError)],
         required=False,
         max_length=150,
         widget=forms.Textarea,

readthedocs/projects/views/base.py

@@ -1,15 +1,12 @@
 """Mix-in classes for project views."""
 import structlog
-from datetime import timedelta
 from functools import lru_cache
 
 from django.conf import settings
 from django.http import HttpResponseRedirect
-from django.shortcuts import get_object_or_404
+from django.shortcuts import render, get_object_or_404
 from django.urls import reverse
-from django.utils import timezone
 
-from readthedocs.projects.exceptions import ProjectSpamError
 from readthedocs.projects.models import Project
 
 log = structlog.get_logger(__name__)
@@ -86,26 +83,17 @@ def get_form(self, data=None, files=None, **kwargs):
 
 class ProjectSpamMixin:
 
-    """Protects POST views from spammers."""
-
-    def post(self, request, *args, **kwargs):
-        log.bind(user_username=request.user.username)
-        if request.user.profile.banned:
-            log.info('Rejecting project POST from shadowbanned user.')
-            return HttpResponseRedirect(self.get_failure_url())
-        try:
-            return super().post(request, *args, **kwargs)
-        except ProjectSpamError:
-            date_maturity = timezone.now() - timedelta(
-                days=settings.USER_MATURITY_DAYS
-            )
-            if request.user.date_joined > date_maturity:
-                request.user.profile.banned = True
-                request.user.profile.save()
-                log.info('Spam detected from new user, shadowbanned user.')
-            else:
-                log.info('Spam detected from user.')
-            return HttpResponseRedirect(self.get_failure_url())
-
-    def get_failure_url(self):
-        return reverse('homepage')
+    """
+    Protects views for spammy projects.
+
+    It shows a ``Project marked as spam`` page and return 410 GONE if the
+    project's dashboard is denied.
+    """
+
+    def get(self, request, *args, **kwargs):
+        if 'readthedocsext.spamfighting' in settings.INSTALLED_APPS:
+            from readthedocsext.spamfighting.utils import is_show_dashboard_denied  # noqa
+            if is_show_dashboard_denied(self.get_project()):
+                return render(request, template_name='spam.html', status=410)
+
+        return super().get(request, *args, **kwargs)

readthedocs/projects/views/mixins.py

@@ -4,12 +4,17 @@
 from celery import chain
 from django.shortcuts import get_object_or_404
 
+import structlog
+
 from readthedocs.core.resolver import resolve, resolve_path
 from readthedocs.core.utils import prepare_build
 from readthedocs.projects.models import Project
 from readthedocs.projects.signals import project_import
 
 
+log = structlog.get_logger(__name__)
+
+
 class ProjectRelationMixin:
 
     """
@@ -117,6 +122,12 @@ def finish_import_project(self, request, project, tags=None):
         for tag in tags:
             project.tags.add(tag)
 
+        log.info(
+            'Project imported.',
+            project_slug=project.slug,
+            user_username=request.user.username,
+        )
+
         # TODO: this signal could be removed, or used for sync task
         project_import.send(sender=project, request=request)
 

readthedocs/projects/views/private.py

@@ -73,7 +73,7 @@
 )
 from readthedocs.projects.notifications import EmailConfirmNotification
 from readthedocs.projects.utils import get_csv_file
-from readthedocs.projects.views.base import ProjectAdminMixin, ProjectSpamMixin
+from readthedocs.projects.views.base import ProjectAdminMixin
 from readthedocs.projects.views.mixins import (
     ProjectImportMixin,
     ProjectRelationListMixin,
@@ -143,7 +143,7 @@ def get_queryset(self):
         return self.model.objects.for_admin_user(self.request.user)
 
 
-class ProjectUpdate(ProjectSpamMixin, ProjectMixin, UpdateView):
+class ProjectUpdate(ProjectMixin, UpdateView):
 
     form_class = UpdateProjectForm
     success_message = _('Project settings updated')
@@ -153,7 +153,7 @@ def get_success_url(self):
         return reverse('projects_detail', args=[self.object.slug])
 
 
-class ProjectAdvancedUpdate(ProjectSpamMixin, ProjectMixin, UpdateView):
+class ProjectAdvancedUpdate(ProjectMixin, UpdateView):
 
     form_class = ProjectAdvancedForm
     success_message = _('Project settings updated')
@@ -253,10 +253,7 @@ def post(self, request, *args, **kwargs):
         return HttpResponseRedirect(self.get_success_url())
 
 
-class ImportWizardView(
-        ProjectImportMixin, ProjectSpamMixin, PrivateViewMixin,
-        SessionWizardView,
-):
+class ImportWizardView(ProjectImportMixin, PrivateViewMixin, SessionWizardView):
 
     """
     Project import wizard.
@@ -287,10 +284,17 @@ def _set_initial_dict(self):
         else:
             self.initial_dict = self.storage.data.get(self.initial_dict_key, {})
 
-    def post(self, *args, **kwargs):
+    def post(self, request, *args, **kwargs):
         self._set_initial_dict()
+
+        log.bind(user_username=request.user.username)
+
+        if request.user.profile.banned:
+            log.info('Rejecting project POST from shadowbanned user.')
+            return HttpResponseRedirect(reverse('homepage'))
+
         # The storage is reset after everything is done.
-        return super().post(*args, **kwargs)
+        return super().post(request, *args, **kwargs)
 
     def get_form_kwargs(self, step=None):
         """Get args to pass into form instantiation."""

readthedocs/projects/views/public.py

@@ -40,7 +40,7 @@
 from readthedocs.storage import build_media_storage
 
 from ..constants import PRIVATE
-from .base import ProjectOnboardMixin
+from .base import ProjectOnboardMixin, ProjectSpamMixin
 
 log = structlog.get_logger(__name__)
 search_log = structlog.get_logger(__name__ + '.search')
@@ -87,8 +87,13 @@ def project_redirect(request, invalid_project_slug):
     ))
 
 
-class ProjectDetailViewBase(ProjectRelationListMixin, BuildTriggerMixin,
-                            ProjectOnboardMixin, DetailView):
+class ProjectDetailViewBase(
+        ProjectSpamMixin,
+        ProjectRelationListMixin,
+        BuildTriggerMixin,
+        ProjectOnboardMixin,
+        DetailView,
+):
 
     """Display project onboard steps."""
 

readthedocs/rtd_tests/tests/test_project_forms.py

@@ -5,7 +5,6 @@
 from django.test.utils import override_settings
 from django.utils.translation import ugettext_lazy as _
 from django_dynamic_fixture import get
-from textclassifier.validators import ClassifierValidator
 
 from readthedocs.builds.constants import EXTERNAL, LATEST, STABLE
 from readthedocs.builds.models import Version
@@ -16,7 +15,6 @@
     REPO_TYPE_HG,
     SPHINX,
 )
-from readthedocs.projects.exceptions import ProjectSpamError
 from readthedocs.projects.forms import (
     EmailHookForm,
     EnvironmentVariableForm,
@@ -36,20 +34,6 @@
 
 class TestProjectForms(TestCase):
 
-    @mock.patch.object(ClassifierValidator, '__call__')
-    def test_form_spam(self, mocked_validator):
-        """Form description field fails spam validation."""
-        mocked_validator.side_effect = ProjectSpamError
-
-        data = {
-            'description': 'foo',
-            'documentation_type': 'sphinx',
-            'language': 'en',
-        }
-        form = ProjectExtraForm(data)
-        with self.assertRaises(ProjectSpamError):
-            form.is_valid()
-
     def test_import_repo_url(self):
         """Validate different type of repository URLs on importing a Project."""
 

readthedocs/rtd_tests/tests/test_project_views.py

@@ -16,7 +16,6 @@
 from readthedocs.oauth.models import RemoteRepository, RemoteRepositoryRelation
 from readthedocs.organizations.models import Organization
 from readthedocs.projects.constants import PUBLIC
-from readthedocs.projects.exceptions import ProjectSpamError
 from readthedocs.projects.models import Domain, Project, WebHook, WebHookEvent
 from readthedocs.projects.views.mixins import ProjectRelationMixin
 from readthedocs.projects.views.private import ImportWizardView
@@ -25,7 +24,7 @@
 
 
 @mock.patch('readthedocs.projects.tasks.update_docs_task', mock.MagicMock())
-class TestProfileMiddleware(RequestFactoryTestMixin, TestCase):
+class TestImportProjectBannedUser(RequestFactoryTestMixin, TestCase):
 
     wizard_class_slug = 'import_wizard_view'
     url = '/dashboard/import/manual/'
@@ -50,26 +49,15 @@ def setUp(self):
                               for (k, v) in list(data[key].items())})
         self.data['{}-current_step'.format(self.wizard_class_slug)] = 'extra'
 
-    def test_profile_middleware_no_profile(self):
+    def test_not_banned_user(self):
         """User without profile and isn't banned."""
         req = self.request(method='post', path='/projects/import', data=self.data)
         req.user = get(User, profile=None)
         resp = ImportWizardView.as_view()(req)
         self.assertEqual(resp.status_code, 302)
         self.assertEqual(resp['location'], '/projects/foobar/')
 
-    @mock.patch('readthedocs.projects.views.private.ProjectBasicsForm.clean')
-    def test_profile_middleware_spam(self, form):
-        """User will be banned."""
-        form.side_effect = ProjectSpamError
-        req = self.request(method='post', path='/projects/import', data=self.data)
-        req.user = get(User)
-        resp = ImportWizardView.as_view()(req)
-        self.assertEqual(resp.status_code, 302)
-        self.assertEqual(resp['location'], '/')
-        self.assertTrue(req.user.profile.banned)
-
-    def test_profile_middleware_banned(self):
+    def test_banned_user(self):
         """User is banned."""
         req = self.request(method='post', path='/projects/import', data=self.data)
         req.user = get(User)
@@ -296,54 +284,6 @@ def test_remote_repository_is_added(self):
         self.assertIsNotNone(proj)
         self.assertEqual(proj.remote_repository, remote_repo)
 
-    @mock.patch(
-        'readthedocs.projects.views.private.ProjectExtraForm.clean_description',
-        create=True,
-    )
-    def test_form_spam(self, mocked_validator):
-        """Don't add project on a spammy description."""
-        self.user.date_joined = timezone.now() - timedelta(days=365)
-        self.user.save()
-        mocked_validator.side_effect = ProjectSpamError
-
-        with self.assertRaises(Project.DoesNotExist):
-            proj = Project.objects.get(name='foobar')
-
-        resp = self.post_step('basics')
-        self.assertWizardResponse(resp, 'extra')
-        resp = self.post_step('extra', session=list(resp._request.session.items()))
-        self.assertIsInstance(resp, HttpResponseRedirect)
-        self.assertEqual(resp.status_code, 302)
-        self.assertEqual(resp['location'], '/')
-
-        with self.assertRaises(Project.DoesNotExist):
-            proj = Project.objects.get(name='foobar')
-        self.assertFalse(self.user.profile.banned)
-
-    @mock.patch(
-        'readthedocs.projects.views.private.ProjectExtraForm.clean_description',
-        create=True,
-    )
-    def test_form_spam_ban_user(self, mocked_validator):
-        """Don't add spam and ban new user."""
-        self.user.date_joined = timezone.now()
-        self.user.save()
-        mocked_validator.side_effect = ProjectSpamError
-
-        with self.assertRaises(Project.DoesNotExist):
-            proj = Project.objects.get(name='foobar')
-
-        resp = self.post_step('basics')
-        self.assertWizardResponse(resp, 'extra')
-        resp = self.post_step('extra', session=list(resp._request.session.items()))
-        self.assertIsInstance(resp, HttpResponseRedirect)
-        self.assertEqual(resp.status_code, 302)
-        self.assertEqual(resp['location'], '/')
-
-        with self.assertRaises(Project.DoesNotExist):
-            proj = Project.objects.get(name='foobar')
-        self.assertTrue(self.user.profile.banned)
-
 
 @mock.patch('readthedocs.core.utils.trigger_build', mock.MagicMock())
 class TestPublicViews(TestCase):

readthedocs/settings/base.py

@@ -170,7 +170,6 @@ def INSTALLED_APPS(self):  # noqa
             'rest_framework',
             'rest_framework.authtoken',
             'corsheaders',
-            'textclassifier',
             'annoying',
             'django_extensions',
             'crispy_forms',

requirements/pip.txt

@@ -82,12 +82,6 @@ django-crispy-forms==1.13.0
 
 docker==5.0.3
 
-django-textclassifier==1.0
-# django-textclassifier doesn't have pinned versions
-# if there is an update they could break our code
-nltk==3.6.6
-textblob==0.17.1
-
 django-annoying==0.10.6
 django-messages-extends==0.6.2
 djangorestframework-jsonp==1.0.2