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

@@ -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)

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"
+        }
     }
 ]

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 @@ from django.forms.util import ErrorList
 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 @@ class UserRegisterForm(forms.ModelForm):
                 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 @@ class UserEditForm(UserRegisterForm):
         super(UserEditForm, self).__init__(*args, **kwargs)
 
     class Meta:
-        model = models.UserProfile
+        model = UserProfile
         exclude = ['password']
 
     def clean(self):
@@ -154,3 +163,22 @@ class UserEditForm(UserRegisterForm):
         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

apps/users/models.py

@@ -191,3 +191,18 @@ class UserProfile(amo.models.ModelBase):
         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()

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() }}
+  <link rel="stylesheet" href="{{ MEDIA_URL }}css/zamboni/admin-django.css">
+{% endblock %}
+
+{% block title %}{{ page_title('Add Blacklisted Nicknames') }}{% endblock %}
+
+{% block bodyclass %}users-blacklistednickname change-form{% endblock %}
+
+{% block content %}
+
+<div class="breadcrumbs">
+  <a href="{{ url('admin:index') }}">Home</a> &rsaquo;
+  <a href="{{ url('admin:app_list', app_label='users') }}">Users</a> &rsaquo;
+  <a href="{{ url('admin:users_blacklistednickname_changelist') }}">Blacklisted nicknames</a> &rsaquo;
+  Add
+</div>
+<div id="content" class="colM">
+  <h2>Add Blacklisted Nicknames</h2>
+  <div id="content-main">
+    {% if messages %}
+      {% for message in messages %}
+        <div class="notification-box {{ message.tags }}">
+          <h2>{{ message }}</h2>
+        </div>
+      {% endfor %}
+    {% endif %}
+
+    {% if form %}
+      <form method="post" action="">
+        {{ csrf() }}
+        <p>Enter one nickname per line.</p>
+        <div class="form-row">
+          {{ form.nicknames.errors|safe }}
+          <label for="id_nicknames">Blacklisted Nicknames {{ required() }}</label>
+          {{ form.nicknames|safe }}
+        </div>
+        <div class="submit-row">
+          <input type="submit" name="_save" class="default" value="Save">
+        </div>
+      </form>
+    {% endif %}
+  </div>
+</div>
+
+{% endblock content %}

apps/users/tests/test_forms.py

@@ -8,6 +8,7 @@ from nose.tools import eq_
 import test_utils
 
 from amo.helpers import urlparams
+from amo.urlresolvers import reverse
 
 
 class UserFormBase(test_utils.TestCase):
@@ -204,8 +205,6 @@ class TestUserLoginForm(UserFormBase):
                                    '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 @@ class TestUserRegisterForm(UserFormBase):
                                             '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 @@ class TestUserRegisterForm(UserFormBase):
         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)

apps/users/tests/test_models.py

@@ -9,7 +9,7 @@ from nose.tools import eq_
 
 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 @@ class TestPasswords(test.TestCase):
         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)

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;