From 26fd705c61be72063a47cceea7a61dc9578d52a3 Mon Sep 17 00:00:00 2001 From: chenba Date: Mon, 12 Jul 2010 20:46:23 -0700 Subject: [PATCH] Bug 574276: Implemented the nickname blacklist. - added the BlacklistedNickname model to the users app - updated the user registration form to check for blacklisted nicknames - admin add form for blacklisted nicknames - tests --- apps/users/admin.py | 39 ++++++++++++- apps/users/fixtures/users/test_backends.json | 58 +++++++++++++++++++ apps/users/forms.py | 34 ++++++++++- apps/users/models.py | 15 +++++ .../admin/blacklisted_nickname/add.html | 48 +++++++++++++++ apps/users/tests/test_forms.py | 32 +++++++++- apps/users/tests/test_models.py | 10 +++- migrations/55-users_blacklistednickname.sql | 8 +++ 8 files changed, 236 insertions(+), 8 deletions(-) create mode 100644 apps/users/templates/admin/blacklisted_nickname/add.html create mode 100644 migrations/55-users_blacklistednickname.sql diff --git a/apps/users/admin.py b/apps/users/admin.py index d2ae742aeac..81413de79de 100644 --- a/apps/users/admin.py +++ b/apps/users/admin.py @@ -1,7 +1,11 @@ -from django.contrib import admin +from django.contrib import admin, messages +from django.db.utils import IntegrityError + +import jingo from access.admin import GroupUserInline -from .models import UserProfile +from .models import UserProfile, BlacklistedNickname +from .users import forms class UserAdmin(admin.ModelAdmin): @@ -32,4 +36,35 @@ class UserAdmin(admin.ModelAdmin): ) +class BlacklistedNicknameAdmin(admin.ModelAdmin): + list_display = search_fields = ('nickname',) + + def add_view(self, request, form_url='', extra_context=None): + """Override the default admin add view for bulk add.""" + form = forms.BlacklistedNicknameAddForm() + if request.method == 'POST': + form = forms.BlacklistedNicknameAddForm(request.POST) + if form.is_valid(): + inserted = 0 + duplicates = 0 + for n in form.cleaned_data['nicknames'].splitlines(): + try: + BlacklistedNickname.objects.create(nickname=n) + inserted += 1 + except IntegrityError: + # note: unless we manage the transactions manually, + # we do lose a primary id here + duplicates += 1 + msg = '%s new nicknames added to the blacklist.' % (inserted) + if duplicates: + msg += ' %s duplicates were ignored.' % (duplicates) + messages.success(request, msg) + # Default django admin change list view does not print messages + # no redirect for now + # return http.HttpResponseRedirect(reverse( + # 'admin:users_blacklistednickname_changelist')) + return jingo.render(request, 'admin/blacklisted_nickname/add.html', + {'form': form}) + admin.site.register(UserProfile, UserAdmin) +admin.site.register(BlacklistedNickname, BlacklistedNicknameAdmin) diff --git a/apps/users/fixtures/users/test_backends.json b/apps/users/fixtures/users/test_backends.json index ac5b6a16f13..194774f62f4 100644 --- a/apps/users/fixtures/users/test_backends.json +++ b/apps/users/fixtures/users/test_backends.json @@ -96,5 +96,63 @@ "modified": "2009-09-24 20:11:30", "notifyevents": 1 } + }, + { + "pk": 5349055, + "model": "auth.user", + "fields": { + "username": "testo@example.com", + "first_name": "Barry", + "last_name": "", + "is_active": true, + "is_superuser": true, + "is_staff": true, + "last_login": "2010-07-23 00:43:32", + "groups": [], + "user_permissions": [], + "password": "sha512$9ec347fc67fc17c99a9dd43d1431a5959011acf247fb2128228c9a04208d1991$edcc677c7045ee2a379286e2ad6d99bd58af6b4a2117e1e4c2743923a9a297ce56d5311fe51baa4584c5483af0eaedc57700fbb4b719c0a84597c1609536ca4d", + "email": "testo@example.com", + "date_joined": "2010-07-03 23:03:11" + } + }, + { + "pk": 5349055, + "model": "users.userprofile", + "fields": { + "sandboxshown": false, + "display_collections_fav": false, + "display_collections": false, + "occupation": "", + "confirmationcode": "", + "location": "", + "picture_type": "", + "averagerating": "", + "homepage": "", + "email": "testo@example.com", + "notifycompat": true, + "bio": 1124105, + "firstname": "Barry", + "deleted": false, + "lastname": "", + "emailhidden": false, + "user": 5349055, + "password": "sha512$9ec347fc67fc17c99a9dd43d1431a5959011acf247fb2128228c9a04208d1991$edcc677c7045ee2a379286e2ad6d99bd58af6b4a2117e1e4c2743923a9a297ce56d5311fe51baa4584c5483af0eaedc57700fbb4b719c0a84597c1609536ca4d", + "nickname": "barry", + "resetcode_expires": "2010-07-23 00:37:10", + "resetcode": "", + "created": "2010-07-03 23:03:11", + "notes": "", + "modified": "2010-07-23 00:37:10", + "notifyevents": true + } + }, + { + "pk": 1, + "model": "users.blacklistednickname", + "fields": { + "nickname": "IE6Fan", + "modified": "2010-07-21 23:32:05", + "created": "2010-07-21 23:32:05" + } } ] diff --git a/apps/users/forms.py b/apps/users/forms.py index d5849de95ba..d17f0bbcb6c 100644 --- a/apps/users/forms.py +++ b/apps/users/forms.py @@ -1,3 +1,5 @@ +import os + from django import forms from django.contrib.auth import forms as auth_forms from django.forms.util import ErrorList @@ -5,7 +7,7 @@ import commonware.log from tower import ugettext as _ -from . import models +from .models import UserProfile, BlacklistedNickname log = commonware.log.getLogger('z.users') @@ -74,7 +76,7 @@ class UserRegisterForm(forms.ModelForm): widget=forms.PasswordInput(render_value=False)) class Meta: - model = models.UserProfile + model = UserProfile def clean_nickname(self): """We're breaking the rules and allowing null=True and blank=True on a @@ -109,6 +111,13 @@ def clean(self): self._errors["lastname"] = ErrorList([msg]) self._errors["nickname"] = ErrorList([msg]) + # Nickname could be blacklisted + if ("nickname" not in self._errors + and nname + and BlacklistedNickname.blocked(nname)): + msg = _("This nickname is invalid.") + self._errors["nickname"] = ErrorList([msg]) + return data @@ -121,7 +130,7 @@ def __init__(self, *args, **kwargs): super(UserEditForm, self).__init__(*args, **kwargs) class Meta: - model = models.UserProfile + model = UserProfile exclude = ['password'] def clean(self): @@ -154,3 +163,22 @@ def save(self): log.debug(u'User (%s) updated their profile' % amouser) amouser.save() + + +class BlacklistedNicknameAddForm(forms.Form): + """Form for adding blacklisted nickname in bulk fashion.""" + nicknames = forms.CharField(widget=forms.Textarea( + attrs={'cols': 40, 'rows': 16})) + + def clean(self): + super(BlacklistedNicknameAddForm, self).clean() + data = self.cleaned_data + + if 'nicknames' in data: + data['nicknames'] = os.linesep.join( + [s for s in data['nicknames'].splitlines() if s]) + if 'nicknames' not in data or data['nicknames'] == '': + msg = 'Please enter at least one nickname to blacklist.' + self._errors['nicknames'] = ErrorList([msg]) + + return data diff --git a/apps/users/models.py b/apps/users/models.py index 5593a5946a4..aa4bcc31da4 100644 --- a/apps/users/models.py +++ b/apps/users/models.py @@ -191,3 +191,18 @@ def create_django_user(self): self.user.save() self.save() return self.user + + +class BlacklistedNickname(amo.models.ModelBase): + """Blacklisted user nicknames.""" + nickname = models.CharField(max_length=255, unique=True, default='') + + def __unicode__(self): + return self.nickname + + @classmethod + def blocked(cls, nick): + """Check to see if a nickname is in the blacklist.""" + # Could also cache the entire blacklist and simply check if the + # nickname is in the list here. @TODO? + return cls.uncached.only('nickname').filter(nickname=nick).exists() diff --git a/apps/users/templates/admin/blacklisted_nickname/add.html b/apps/users/templates/admin/blacklisted_nickname/add.html new file mode 100644 index 00000000000..64fa6a8b04c --- /dev/null +++ b/apps/users/templates/admin/blacklisted_nickname/add.html @@ -0,0 +1,48 @@ +{% extends "admin/base.html" %} +{% from 'includes/forms.html' import required %} +{% block extrahead %} + {{ super() }} + +{% endblock %} + +{% block title %}{{ page_title('Add Blacklisted Nicknames') }}{% endblock %} + +{% block bodyclass %}users-blacklistednickname change-form{% endblock %} + +{% block content %} + +
+ Home › + Users › + Blacklisted nicknames › + Add +
+
+

Add Blacklisted Nicknames

+
+ {% if messages %} + {% for message in messages %} +
+

{{ message }}

+
+ {% endfor %} + {% endif %} + + {% if form %} +
+ {{ csrf() }} +

Enter one nickname per line.

+
+ {{ form.nicknames.errors|safe }} + + {{ form.nicknames|safe }} +
+
+ +
+
+ {% endif %} +
+
+ +{% endblock content %} diff --git a/apps/users/tests/test_forms.py b/apps/users/tests/test_forms.py index 0f76a1a0e1d..4e4ee0edcd6 100644 --- a/apps/users/tests/test_forms.py +++ b/apps/users/tests/test_forms.py @@ -8,6 +8,7 @@ import test_utils from amo.helpers import urlparams +from amo.urlresolvers import reverse class UserFormBase(test_utils.TestCase): @@ -204,8 +205,6 @@ def test_redirect_after_login_evil(self): 'password': 'foo'}, follow=True) self.assertRedirects(r, '/en-US/firefox/') - - def test_unconfirmed_account(self): url = self._get_login_url() self.user_profile.confirmationcode = 'blah' @@ -262,6 +261,15 @@ def test_set_unmatched_passwords(self): 'The passwords did not match.') eq_(len(mail.outbox), 0) + def test_invalid_nickname(self): + data = {'email': 'testo@example.com', + 'password': 'xxx', + 'password2': 'xxx', + 'nickname': 'IE6Fan', } + r = self.client.post('/en-US/firefox/users/register', data) + self.assertFormError(r, 'form', 'nickname', + 'This nickname is invalid.') + def test_already_logged_in(self): self.client.login(username='jbalogh@mozilla.com', password='foo') r = self.client.get('/users/register', follow=True) @@ -286,3 +294,23 @@ def test_success(self): assert mail.outbox[0].subject.find('Please confirm your email') == 0 assert mail.outbox[0].body.find('%s/confirm/%s' % (u.id, u.confirmationcode)) > 0 + + +class TestBlacklistedNicknameAdminAddForm(UserFormBase): + + def test_no_nicknames(self): + self.client.login(username='testo@example.com', password='foo') + url = reverse('admin:users_blacklistednickname_add') + data = {'nicknames': "\n\n", } + r = self.client.post(url, data) + msg = 'Please enter at least one nickname to blacklist.' + self.assertFormError(r, 'form', 'nicknames', msg) + + def test_add(self): + self.client.login(username='testo@example.com', password='foo') + url = reverse('admin:users_blacklistednickname_add') + data = {'nicknames': "IE6Fan\nfubar\n\n", } + r = self.client.post(url, data) + msg = '1 new nicknames added to the blacklist. ' + msg += '1 duplicates were ignored.' + self.assertContains(r, msg) diff --git a/apps/users/tests/test_models.py b/apps/users/tests/test_models.py index 202e196dae1..9483f32e1dc 100644 --- a/apps/users/tests/test_models.py +++ b/apps/users/tests/test_models.py @@ -9,7 +9,7 @@ from addons.models import Addon, AddonUser from reviews.models import Review -from users.models import UserProfile, get_hexdigest +from users.models import UserProfile, get_hexdigest, BlacklistedNickname class TestUserProfile(test.TestCase): @@ -141,3 +141,11 @@ def test_valid_new_password(self): u = UserProfile() u.set_password('sekrit') assert u.check_password('sekrit') is True + + +class TestBlacklistedNickname(test.TestCase): + fixtures = ['users/test_backends'] + + def test_blocked(self): + eq_(BlacklistedNickname.blocked('IE6Fan'), True) + eq_(BlacklistedNickname.blocked('testo'), False) diff --git a/migrations/55-users_blacklistednickname.sql b/migrations/55-users_blacklistednickname.sql new file mode 100644 index 00000000000..5950c62d075 --- /dev/null +++ b/migrations/55-users_blacklistednickname.sql @@ -0,0 +1,8 @@ +CREATE TABLE `users_blacklistednickname` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `created` datetime NOT NULL, + `modified` datetime NOT NULL, + `nickname` varchar(255) NOT NULL, + PRIMARY KEY (`id`), + UNIQUE KEY `nickname` (`nickname`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8;