From f9165ed4080f3a7395011210384eab5573eb06f3 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Sun, 22 Nov 2015 17:45:34 -0800 Subject: [PATCH] Add some spam protection features This adds validation to the project description field --- .../core/migrations/0003_add_banned_status.py | 19 ++++++ readthedocs/core/models.py | 1 + readthedocs/projects/exceptions.py | 7 ++ readthedocs/projects/forms.py | 7 ++ readthedocs/projects/urls/private.py | 5 +- readthedocs/projects/views/base.py | 38 ++++++++++- readthedocs/projects/views/private.py | 68 +++++++------------ .../rtd_tests/tests/test_project_forms.py | 24 +++++++ .../rtd_tests/tests/test_project_views.py | 46 ++++++++++++- readthedocs/settings/base.py | 1 + requirements/pip.txt | 1 + 11 files changed, 169 insertions(+), 48 deletions(-) create mode 100644 readthedocs/core/migrations/0003_add_banned_status.py create mode 100644 readthedocs/rtd_tests/tests/test_project_forms.py diff --git a/readthedocs/core/migrations/0003_add_banned_status.py b/readthedocs/core/migrations/0003_add_banned_status.py new file mode 100644 index 000000000..5c9d4ab79 --- /dev/null +++ b/readthedocs/core/migrations/0003_add_banned_status.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0002_make_userprofile_user_a_onetoonefield'), + ] + + operations = [ + migrations.AddField( + model_name='userprofile', + name='banned', + field=models.BooleanField(default=False, verbose_name='Banned'), + ), + ] diff --git a/readthedocs/core/models.py b/readthedocs/core/models.py index 55e8e62d7..83e23ea8d 100644 --- a/readthedocs/core/models.py +++ b/readthedocs/core/models.py @@ -18,6 +18,7 @@ class UserProfile (models.Model): user = models.OneToOneField('auth.User', verbose_name=_('User'), related_name='profile') whitelisted = models.BooleanField(_('Whitelisted'), default=False) + banned = models.BooleanField(_('Banned'), default=False) homepage = models.CharField(_('Homepage'), max_length=100, blank=True) allow_email = models.BooleanField(_('Allow email'), help_text=_('Show your email on VCS ' diff --git a/readthedocs/projects/exceptions.py b/readthedocs/projects/exceptions.py index 23e23d429..9fa628f88 100644 --- a/readthedocs/projects/exceptions.py +++ b/readthedocs/projects/exceptions.py @@ -6,3 +6,10 @@ class ProjectImportError (Exception): """Failure to import a project from a repository.""" pass + + +class ProjectSpamError(Exception): + + """Error raised when a project field has detected spam""" + + pass diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 60c0c7e49..369b46242 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -10,6 +10,7 @@ from django.template.defaultfilters import slugify from django.template.loader import render_to_string from django.utils.translation import ugettext_lazy as _ from django.utils.safestring import mark_safe +from textclassifier.validators import ClassifierValidator from guardian.shortcuts import assign @@ -17,6 +18,7 @@ from readthedocs.builds.constants import TAG from readthedocs.core.utils import trigger_build from readthedocs.redirects.models import Redirect from readthedocs.projects import constants +from readthedocs.projects.exceptions import ProjectSpamError from readthedocs.projects.models import Project, EmailHook, WebHook, Domain from readthedocs.privacy.loader import AdminPermission @@ -130,6 +132,11 @@ class ProjectExtraForm(ProjectForm): 'tags', ) + description = forms.CharField( + validators=[ClassifierValidator(raises=ProjectSpamError)], + widget=forms.Textarea + ) + class ProjectAdvancedForm(ProjectTriggerBuildMixin, ProjectForm): diff --git a/readthedocs/projects/urls/private.py b/readthedocs/projects/urls/private.py index d912407ca..174802f95 100644 --- a/readthedocs/projects/urls/private.py +++ b/readthedocs/projects/urls/private.py @@ -4,6 +4,7 @@ from django.conf.urls import patterns, url from readthedocs.projects.views.private import ( ProjectDashboard, ImportView, + ProjectUpdate, ProjectAdvancedUpdate, DomainList, DomainCreate, DomainDelete, DomainUpdate) from readthedocs.projects.backends.views import ImportWizardView, ImportDemoView @@ -37,11 +38,11 @@ urlpatterns = patterns( name='projects_comments_moderation'), url(r'^(?P[-\w]+)/edit/$', - 'readthedocs.projects.views.private.project_edit', + ProjectUpdate.as_view(), name='projects_edit'), url(r'^(?P[-\w]+)/advanced/$', - 'readthedocs.projects.views.private.project_advanced', + ProjectAdvancedUpdate.as_view(), name='projects_advanced'), url(r'^(?P[-\w]+)/version/(?P[^/]+)/delete_html/$', diff --git a/readthedocs/projects/views/base.py b/readthedocs/projects/views/base.py index 6cd2a1158..30d816208 100644 --- a/readthedocs/projects/views/base.py +++ b/readthedocs/projects/views/base.py @@ -1,7 +1,17 @@ +import logging +from datetime import datetime, timedelta + +from django.conf import settings from django.shortcuts import get_object_or_404 from django.core.urlresolvers import reverse +from django.http import HttpResponseRedirect -from readthedocs.projects.models import Project +from ..models import Project +from ..exceptions import ProjectSpamError + +log = logging.getLogger(__name__) + +USER_MATURITY_DAYS = getattr(settings, 'USER_MATURITY_DAYS', 7) class ProjectOnboardMixin(object): @@ -71,3 +81,29 @@ class ProjectAdminMixin(object): def get_success_url(self, **kwargs): return reverse('projects_domains', args=[self.get_project().slug]) + + +class ProjectSpamMixin(object): + + """Protects POST views from spammers""" + + def post(self, request, *args, **kwargs): + if request.user.profile.banned: + log.error('Rejecting project POST from shadowbanned user %s', + request.user) + return HttpResponseRedirect(self.get_failure_url()) + try: + return super(ProjectSpamMixin, self).post(request, *args, **kwargs) + except ProjectSpamError: + date_maturity = datetime.now() - timedelta(days=USER_MATURITY_DAYS) + if request.user.date_joined > date_maturity: + request.user.profile.banned = True + request.user.profile.save() + log.error('Spam detected from new user, shadowbanned user %s', + request.user) + else: + log.error('Spam detected from user %s', request.user) + return HttpResponseRedirect(self.get_failure_url()) + + def get_failure_url(self): + return reverse('homepage') diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 415102f97..c43e123f9 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -32,8 +32,9 @@ from readthedocs.projects.forms import ( build_versions_form, UserForm, EmailHookForm, TranslationForm, RedirectForm, WebHookForm, DomainForm) from readthedocs.projects.models import Project, EmailHook, WebHook, Domain -from readthedocs.projects.views.base import ProjectAdminMixin +from readthedocs.projects.views.base import ProjectAdminMixin, ProjectSpamMixin from readthedocs.projects import constants, tasks +from readthedocs.projects.exceptions import ProjectSpamError from readthedocs.projects.tasks import remove_dir, clear_artifacts from readthedocs.core.mixins import LoginRequiredMixin @@ -95,58 +96,37 @@ def project_comments_moderation(request, project_slug): {'project': project}) -@login_required -def project_edit(request, project_slug): - """Project edit view - - Edit an existing project - depending on what type of project is being - edited (created or imported) a different form will be displayed - """ - project = get_object_or_404(Project.objects.for_admin_user(request.user), - slug=project_slug) +class ProjectUpdate(ProjectSpamMixin, PrivateViewMixin, UpdateView): form_class = UpdateProjectForm + model = Project + success_message = _('Project settings updated') + template_name = 'projects/project_edit.html' + lookup_url_kwarg = 'project_slug' + lookup_field = 'slug' - form = form_class(instance=project, data=request.POST or None, - user=request.user) + def get_queryset(self): + return self.model.objects.for_admin_user(self.request.user) - if request.method == 'POST' and form.is_valid(): - form.save() - messages.success(request, _('Project settings updated')) - project_dashboard = reverse('projects_detail', args=[project.slug]) - return HttpResponseRedirect(project_dashboard) - - return render_to_response( - 'projects/project_edit.html', - {'form': form, 'project': project}, - context_instance=RequestContext(request) - ) + def get_success_url(self): + return reverse('projects_detail', args=[self.object.slug]) -@login_required -def project_advanced(request, project_slug): - """Project advanced admin view +class ProjectAdvancedUpdate(ProjectSpamMixin, PrivateViewMixin, UpdateView): - Edit an existing project - depending on what type of project is being - edited (created or imported) a different form will be displayed - """ - project = get_object_or_404(Project.objects.for_admin_user(request.user), - slug=project_slug) form_class = ProjectAdvancedForm - form = form_class(instance=project, data=request.POST or None, initial={ - 'num_minor': 2, 'num_major': 2, 'num_point': 2}) + model = Project + success_message = _('Project settings updated') + template_name = 'projects/project_advanced.html' + lookup_url_kwarg = 'project_slug' + lookup_field = 'slug' + initial = {'num_minor': 2, 'num_major': 2, 'num_point': 2} - if request.method == 'POST' and form.is_valid(): - form.save() - messages.success(request, _('Project settings updated')) - project_dashboard = reverse('projects_detail', args=[project.slug]) - return HttpResponseRedirect(project_dashboard) + def get_queryset(self): + return self.model.objects.for_admin_user(self.request.user) - return render_to_response( - 'projects/project_advanced.html', - {'form': form, 'project': project}, - context_instance=RequestContext(request) - ) + def get_success_url(self): + return reverse('projects_detail', args=[self.object.slug]) @login_required @@ -240,7 +220,7 @@ def project_delete(request, project_slug): ) -class ImportWizardView(PrivateViewMixin, SessionWizardView): +class ImportWizardView(ProjectSpamMixin, PrivateViewMixin, SessionWizardView): """Project import wizard""" diff --git a/readthedocs/rtd_tests/tests/test_project_forms.py b/readthedocs/rtd_tests/tests/test_project_forms.py new file mode 100644 index 000000000..afa15a44d --- /dev/null +++ b/readthedocs/rtd_tests/tests/test_project_forms.py @@ -0,0 +1,24 @@ +import mock + +from django.test import TestCase, override_settings +from textclassifier.validators import ClassifierValidator + +from readthedocs.projects.exceptions import ProjectSpamError +from readthedocs.projects.forms import ProjectExtraForm + + +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() diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index c871b16aa..5ffc999b2 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -1,11 +1,15 @@ +from datetime import datetime, timedelta + +import mock +from mock import patch from django.contrib.auth.models import User from django.contrib.messages import constants as message_const from django_dynamic_fixture import get from django_dynamic_fixture import new -from mock import patch from readthedocs.rtd_tests.base import WizardTestCase, MockBuildTestCase from readthedocs.projects.models import Project +from readthedocs.projects.exceptions import ProjectSpamError class TestBasicsForm(WizardTestCase): @@ -81,6 +85,46 @@ class TestAdvancedForm(TestBasicsForm): self.assertWizardFailure(resp, 'language') self.assertWizardFailure(resp, 'documentation_type') + @patch('readthedocs.projects.forms.ProjectExtraForm.clean_description', + create=True) + def test_form_spam(self, mocked_validator): + '''Don't add project on a spammy description''' + self.eric.date_joined = datetime.now() - timedelta(days=365) + self.eric.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') + self.assertWizardResponse(resp) + + with self.assertRaises(Project.DoesNotExist): + proj = Project.objects.get(name='foobar') + self.assertFalse(self.eric.profile.banned) + + @patch('readthedocs.projects.forms.ProjectExtraForm.clean_description', + create=True) + def test_form_spam_ban_user(self, mocked_validator): + '''Don't add spam and ban new user''' + self.eric.date_joined = datetime.now() + self.eric.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') + self.assertWizardResponse(resp) + + with self.assertRaises(Project.DoesNotExist): + proj = Project.objects.get(name='foobar') + self.assertTrue(self.eric.profile.banned) + class TestImportDemoView(MockBuildTestCase): '''Test project import demo view''' diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index d741b12fc..67be1a6a1 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -193,6 +193,7 @@ INSTALLED_APPS = [ 'rest_framework', 'corsheaders', 'copyright', + 'textclassifier', # Celery bits 'djcelery', diff --git a/requirements/pip.txt b/requirements/pip.txt index 0b9eec3bf..0a3b118dd 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -52,6 +52,7 @@ django-copyright==1.0.0 django-formtools==1.0 django-dynamic-fixture==1.8.5 docker-py==1.3.1 +django-textclassifier==1.0 # Docs sphinx-http-domain==0.2