Spam: deny dashboard on spammy projects

Quick/Initial implementation to deny showing a project's dashboard if spam score
is above the threshold.

- the old and non-used `ProjectSpamError` that checked for an invalid
  description was removed together with all its logic
- the mixin `ProjectSpamMixin` was re-purposed to check the spam score and deny
  serving the dashboard
- move "disable banned user to import projects" inside the `ImportWizardView`

Note that currently we are only denying the dashboard for the project's
detail ("Overview" in our UI) view. If we want to deny other pages like
"Downloads", "Search", "Builds", "Versions" and "Admin" we need to adapt other
views as well (e.g. migrate them from function-based view to class-based view to
be able to re-use the mixin). This can be implemented in a future version.
Together with this, we could only deny the dashboard to non-maintainers of the
project itself, allowing them to make some changes in case of a mistake.

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

@@ -78,12 +78,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