diff --git a/pytest.ini b/pytest.ini index d06623bde72..1c2105b6973 100644 --- a/pytest.ini +++ b/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 diff --git a/readthedocs/projects/exceptions.py b/readthedocs/projects/exceptions.py index 18841d6de40..3dccdad78b5 100644 --- a/readthedocs/projects/exceptions.py +++ b/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. - """ diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 63e2a88837d..defd16b08da 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -11,7 +11,6 @@ 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 @@ -19,7 +18,6 @@ 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, diff --git a/readthedocs/projects/views/base.py b/readthedocs/projects/views/base.py index 2a7a4de5bb4..ba76f7c137f 100644 --- a/readthedocs/projects/views/base.py +++ b/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) diff --git a/readthedocs/projects/views/mixins.py b/readthedocs/projects/views/mixins.py index 4be46797228..15e9586bc5c 100644 --- a/readthedocs/projects/views/mixins.py +++ b/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) diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 1ecb371fa69..3b559c06a90 100644 --- a/readthedocs/projects/views/private.py +++ b/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.""" diff --git a/readthedocs/projects/views/public.py b/readthedocs/projects/views/public.py index a4e895570e5..2a7eec6c036 100644 --- a/readthedocs/projects/views/public.py +++ b/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.""" diff --git a/readthedocs/rtd_tests/tests/test_project_forms.py b/readthedocs/rtd_tests/tests/test_project_forms.py index 11e52d8745f..4e6097b92cd 100644 --- a/readthedocs/rtd_tests/tests/test_project_forms.py +++ b/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.""" diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index 33768487bb7..ed0030e9f2c 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/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,7 +49,7 @@ 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) @@ -58,18 +57,7 @@ def test_profile_middleware_no_profile(self): 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): diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 226fab5035f..cb7d5ff2816 100644 --- a/readthedocs/settings/base.py +++ b/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', diff --git a/requirements/pip.txt b/requirements/pip.txt index 1c644f11049..9c6782525bf 100644 --- a/requirements/pip.txt +++ b/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