Usernames created w/ browser ID are more robust (bug 708731)

apps/users/tests/test_user_utils.py

@@ -1,7 +1,12 @@
+import fudge
+import mock
 from nose.tools import eq_
 
+from django.conf import settings
+
 import amo.tests
-from users.utils import EmailResetCode
+from users.models import BlacklistedUsername
+from users.utils import EmailResetCode, autocreate_username
 
 
 class TestEmailResetCode(amo.tests.TestCase):
@@ -18,3 +23,43 @@ class TestEmailResetCode(amo.tests.TestCase):
         # A bad token or hash raises ValueError
         self.assertRaises(ValueError, EmailResetCode.parse, token, hash[:-5])
         self.assertRaises(ValueError, EmailResetCode.parse, token[5:], hash)
+
+
+class TestAutoCreateUsername(amo.tests.TestCase):
+
+    def test_invalid_characters(self):
+        eq_(autocreate_username('testaccount+slug'),
+            'testaccountslug')
+
+    def test_blacklisted(self):
+        BlacklistedUsername.objects.create(username='firefox')
+        un = autocreate_username('firefox')
+        assert un != 'firefox', 'Unexpected: %s' % un
+
+    def test_too_long(self):
+        un = autocreate_username('f' + 'u' * 255)
+        assert not un.startswith('fuuuuuuuuuuuuuuuuuu'), 'Unexpected: %s' % un
+
+    @mock.patch.object(settings, 'MAX_GEN_USERNAME_TRIES', 3)
+    @fudge.patch('users.utils.UserProfile.objects.filter')
+    def test_too_many_tries(self, filter):
+        filter = (filter.is_callable().returns_fake().provides('count')
+                  .returns(1))
+        for i in range(3):
+            # Simulate existing username.
+            filter = filter.next_call().returns(1)
+        # Simulate available username.
+        filter = filter.next_call().returns(0)
+        # After the third try, give up, and generate a random string username.
+        un = autocreate_username('base')
+        assert not un.startswith('base'), 'Unexpected: %s' % un
+
+    @fudge.patch('users.utils.UserProfile.objects.filter')
+    def test_duplicate_username_counter(self, filter):
+        filter = (filter.is_callable().returns_fake().provides('count')
+                  .returns(1)
+                  .next_call()
+                  .returns(1)
+                  .next_call()
+                  .returns(0))
+        eq_(autocreate_username('existingname'), 'existingname3')

apps/users/tests/test_views.py

@@ -698,6 +698,21 @@ class TestLogin(UserViewBase):
         self.client.get(self.url)
         assert login.called
 
+    @patch.object(waffle, 'switch_is_active', lambda x: True)
+    @patch('httplib2.Http.request')
+    def test_browserid_duplicate_username(self, http_request):
+        email = 'jbalogh@example.com'  # existing
+        http_request.return_value = (200, json.dumps({'status': 'okay',
+                                                      'email': email}))
+        res = self.client.post(reverse('users.browserid_login'),
+                               data=dict(assertion='fake-assertion',
+                                         audience='fakeamo.org'))
+        eq_(res.status_code, 200)
+        profiles = UserProfile.objects.filter(email=email)
+        eq_(profiles[0].username, 'jbalogh2')
+        # Note: lower level unit tests for this functionality are in
+        # TestAutoCreateUsername()
+
 
 class TestProfileCompletion(UserViewBase):
     fixtures = ['users/test_backends', 'base/addon_3615']

apps/users/utils.py

@@ -1,13 +1,15 @@
 import base64
+from functools import partial
 import hashlib
 import time
+import uuid
 
 from django.conf import settings
 from django.db.models import Q
 
 import commonware.log
 
-from users.models import UserProfile
+from users.models import UserProfile, BlacklistedUsername
 
 log = commonware.log.getLogger('z.users')
 
@@ -100,3 +102,22 @@ def find_users(email):
     """
     return UserProfile.objects.filter(Q(email=email) |
                                       Q(history__email=email)).distinct()
+
+
+def autocreate_username(candidate, tries=1):
+    """Returns a unique valid username."""
+    max_tries = settings.MAX_GEN_USERNAME_TRIES
+    from amo.utils import slugify, SLUG_OK
+    make_u = partial(slugify, ok=SLUG_OK, lower=True, spaces=False,
+                     delimiter='-')
+    adjusted_u = make_u(candidate)
+    if tries > 1:
+        adjusted_u = '%s%s' % (adjusted_u, tries)
+    if (BlacklistedUsername.blocked(adjusted_u)
+        or tries > max_tries or len(adjusted_u) > 255):
+        log.info('username blocked, max tries reached, or too long;'
+                 ' username=%s; max=%s' % (adjusted_u, max_tries))
+        return autocreate_username(uuid.uuid4().hex[0:15])
+    if UserProfile.objects.filter(username=adjusted_u).count():
+        return autocreate_username(candidate, tries=tries + 1)
+    return adjusted_u

apps/users/views.py

@@ -50,7 +50,7 @@ import users.notifications as notifications
 from .models import UserProfile
 from .signals import logged_out
 from . import forms
-from .utils import EmailResetCode, UnsubscribeCode
+from .utils import EmailResetCode, UnsubscribeCode, autocreate_username
 import tasks
 
 log = commonware.log.getLogger('z.users')
@@ -298,7 +298,7 @@ def browserid_authenticate(request, assertion):
     if len(users) == 1:
         users[0].user.backend = 'django_browserid.auth.BrowserIDBackend'
         return (users[0], None)
-    username = email.partition('@')[0]
+    username = autocreate_username(email.partition('@')[0])
     if (settings.REGISTER_USER_LIMIT and
         UserProfile.objects.count() > settings.REGISTER_USER_LIMIT
         and not can_override_reg_limit(request)):

settings.py

@@ -908,6 +908,10 @@ LOGIN_URL = "/users/login"
 LOGOUT_URL = "/users/logout"
 LOGIN_REDIRECT_URL = "/"
 LOGOUT_REDIRECT_URL = "/"
+# When logging in with browser ID, a username is created automatically.
+# In the case of duplicates, the process is recursive up to this number
+# of times.
+MAX_GEN_USERNAME_TRIES = 50
 
 # Legacy Settings
 # used by old-style CSRF token