From fc856dd119b42852a5a91effeffd9b6338d6b279 Mon Sep 17 00:00:00 2001 From: Ee Durbin Date: Mon, 20 Jun 2022 15:45:48 -0400 Subject: [PATCH] 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 --- users/forms.py | 12 ++++++++++++ users/models.py | 10 ++++++++-- users/tests/test_forms.py | 17 +++++++++++++++++ users/tests/test_views.py | 7 ++++--- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/users/forms.py b/users/forms.py index 042b11bb3..fb05144f3 100644 --- a/users/forms.py +++ b/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 --git a/users/models.py b/users/models.py index 0733e23f6..d80f5ceef 100644 --- a/users/models.py +++ b/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 --git a/users/tests/test_forms.py b/users/tests/test_forms.py index 444437e25..10ab95e32 100644 --- a/users/tests/test_forms.py +++ b/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 --git a/users/tests/test_views.py b/users/tests/test_views.py index 02edc77c4..952425c98 100644 --- a/users/tests/test_views.py +++ b/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,