allow username change, avoid new collisions on case insensitive match (#2061)

* allow username change, avoid new collisions on case insensitive match

* fix existing profile edit test and test this feature

* fix tests implicitly getting username from object

* now a required field on user profile edit form

Author: ewdurbin

Diff for: users/forms.py

@@ -9,6 +9,7 @@ class UserProfileForm(ModelForm):
     class Meta:
         model = User
         fields = [
+            'username',
             'first_name',
             'last_name',
             'email',
@@ -22,6 +23,17 @@ class Meta:
             'email_privacy': forms.RadioSelect,
         }
 
+    def clean_username(self):
+        try:
+            user = User.objects.get_by_natural_key(self.cleaned_data.get('username'))
+        except User.MultipleObjectsReturned:
+            raise forms.ValidationError('A user with that username already exists.')
+        except User.DoesNotExist:
+            return self.cleaned_data.get('username')
+        if user == self.instance:
+            return self.cleaned_data.get('username')
+        raise forms.ValidationError('A user with that username already exists.')
+
     def clean_email(self):
         email = self.cleaned_data.get('email')
         user = User.objects.filter(email=email).exclude(pk=self.instance.pk)

Diff for: users/models.py

@@ -1,7 +1,7 @@
 import datetime
 
 from django.conf import settings
-from django.contrib.auth.models import AbstractUser
+from django.contrib.auth.models import AbstractUser, UserManager
 from django.urls import reverse
 from django.db import models
 from django.utils import timezone
@@ -15,6 +15,12 @@
 DEFAULT_MARKUP_TYPE = getattr(settings, 'DEFAULT_MARKUP_TYPE', 'markdown')
 
 
+class CustomUserManager(UserManager):
+    def get_by_natural_key(self, username):
+        case_insensitive_username_field = '{}__iexact'.format(self.model.USERNAME_FIELD)
+        return self.get(**{case_insensitive_username_field: username})
+
+
 class User(AbstractUser):
     bio = MarkupField(blank=True, default_markup_type=DEFAULT_MARKUP_TYPE, escape_html=True)
 
@@ -38,7 +44,7 @@ class User(AbstractUser):
 
     public_profile = models.BooleanField('Make my profile public', default=True)
 
-    objects = UserManager()
+    objects = CustomUserManager()
 
     def get_absolute_url(self):
         return reverse('users:user_detail', kwargs={'slug': self.username})

Diff for: users/tests/test_forms.py

@@ -125,6 +125,7 @@ def test_unique_email(self):
         User.objects.create_user('test42', 'test42@example.com', 'testpass')
 
         form = UserProfileForm({
+            'username': 'stanne',
             'email': 'test42@example.com',
             'search_visibility': 0,
             'email_privacy': 0,
@@ -134,3 +135,19 @@ def test_unique_email(self):
             form.errors,
             {'email': ['Please use a unique email address.']}
         )
+
+    def test_case_insensitive_unique_username(self):
+        User.objects.create_user('stanne', 'mikael@darktranquillity.com', 'testpass')
+        User.objects.create_user('test42', 'test42@example.com', 'testpass')
+
+        form = UserProfileForm({
+            'username': 'Test42',
+            'email': 'mikael@darktranquillity.com',
+            'search_visibility': 0,
+            'email_privacy': 0,
+        }, instance=User.objects.get(username='stanne'))
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            form.errors,
+            {'username': ['A user with that username already exists.']}
+        )

Diff for: users/tests/test_views.py

@@ -84,7 +84,7 @@ def test_membership_update(self):
         self.assertEqual(response.status_code, 302)  # Requires login now
 
         self.assertTrue(self.user2.has_membership)
-        self.client.login(username=self.user2, password='password')
+        self.client.login(username=self.user2.username, password='password')
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
 
@@ -105,7 +105,7 @@ def test_membership_update(self):
     def test_membership_update_404(self):
         url = reverse('users:user_membership_edit')
         self.assertFalse(self.user.has_membership)
-        self.client.login(username=self.user, password='password')
+        self.client.login(username=self.user.username, password='password')
         response = self.client.get(url)
         self.assertEqual(response.status_code, 404)
 
@@ -114,7 +114,7 @@ def test_user_has_already_have_membership(self):
         # has membership.
         url = reverse('users:user_membership_create')
         self.assertTrue(self.user2.has_membership)
-        self.client.login(username=self.user2, password='password')
+        self.client.login(username=self.user2.username, password='password')
         response = self.client.get(url)
         self.assertRedirects(response, reverse('users:user_membership_edit'))
 
@@ -139,6 +139,7 @@ def test_user_update_redirect(self):
 
         # should return 200 if the user does want to see their user profile
         post_data = {
+            'username': 'username',
             'search_visibility': 0,
             'email_privacy': 1,
             'public_profile': False,