From 974590ad5715f46c91b1115c13fef853d12c8e0f Mon Sep 17 00:00:00 2001 From: bohare Date: Thu, 9 Mar 2017 16:44:21 +0000 Subject: [PATCH] Fix #946: Make usernames case insensitive --- cadasta/accounts/forms.py | 22 +++++-- cadasta/accounts/serializers.py | 17 +++++ cadasta/accounts/tests/test_forms.py | 75 ++++++++++++++++++++-- cadasta/accounts/tests/test_serializers.py | 38 +++++++++++ cadasta/accounts/validators.py | 10 +-- 5 files changed, 146 insertions(+), 16 deletions(-) diff --git a/cadasta/accounts/forms.py b/cadasta/accounts/forms.py index 491347d0b..0ba88e215 100644 --- a/cadasta/accounts/forms.py +++ b/cadasta/accounts/forms.py @@ -23,6 +23,14 @@ class Meta: def clean_username(self): username = self.data.get('username') + usernames = [ + u.casefold() for u in + User.objects.values_list('username', flat=True) + ] + if username.casefold() in usernames: + raise forms.ValidationError( + _("A user with that username already exists") + ) if username.lower() in settings.CADASTA_INVALID_ENTITY_NAMES: raise forms.ValidationError( _("Username cannot be “add” or “new”.")) @@ -76,10 +84,15 @@ def __init__(self, *args, **kwargs): def clean_username(self): username = self.data.get('username') - if (self.instance.username != username and - User.objects.filter(username=username).exists()): - raise forms.ValidationError( - _("Another user with this username already exists")) + if self.instance.username.casefold() != username.casefold(): + usernames = [ + u.casefold() for u in + User.objects.values_list('username', flat=True) + ] + if username.casefold() in usernames: + raise forms.ValidationError( + _("A user with that username already exists") + ) if username.lower() in settings.CADASTA_INVALID_ENTITY_NAMES: raise forms.ValidationError( _("Username cannot be “add” or “new”.")) @@ -97,7 +110,6 @@ def clean_email(self): def save(self, *args, **kwargs): user = super().save(commit=False, *args, **kwargs) - if self._send_confirmation: send_email_confirmation(self.request, user) self._send_confirmation = False diff --git a/cadasta/accounts/serializers.py b/cadasta/accounts/serializers.py index 0d0619c89..38c7eaf86 100644 --- a/cadasta/accounts/serializers.py +++ b/cadasta/accounts/serializers.py @@ -35,6 +35,14 @@ class Meta: } def validate_username(self, username): + usernames = [ + u.casefold() for u in + User.objects.values_list('username', flat=True) + ] + if username.casefold() in usernames: + raise ValidationError( + _("A user with that username already exists") + ) if username.lower() in settings.CADASTA_INVALID_ENTITY_NAMES: raise ValidationError( _("Username cannot be “add” or “new”.")) @@ -90,6 +98,15 @@ def validate_username(self, username): username != instance.username and self.context['request'].user != instance): raise ValidationError('Cannot update username') + if instance.username.casefold() != username.casefold(): + usernames = [ + u.casefold() for u in + User.objects.values_list('username', flat=True) + ] + if username.casefold() in usernames: + raise ValidationError( + _("A user with that username already exists") + ) if username.lower() in settings.CADASTA_INVALID_ENTITY_NAMES: raise ValidationError( _("Username cannot be “add” or “new”.")) diff --git a/cadasta/accounts/tests/test_forms.py b/cadasta/accounts/tests/test_forms.py index 3e0794c76..d280665fe 100644 --- a/cadasta/accounts/tests/test_forms.py +++ b/cadasta/accounts/tests/test_forms.py @@ -1,15 +1,14 @@ import random -from django.utils.translation import gettext as _ -from django.test import TestCase +from core.tests.utils.cases import UserTestCase +from django.contrib.messages.storage.fallback import FallbackStorage from django.core import mail from django.http import HttpRequest -from django.contrib.messages.storage.fallback import FallbackStorage +from django.test import TestCase +from django.utils.translation import gettext as _ -from ..models import User from .. import forms -from core.tests.utils.cases import UserTestCase - +from ..models import User from .factories import UserFactory @@ -31,6 +30,37 @@ def test_valid_data(self): user = User.objects.first() assert user.check_password('iloveyoko79!') is True + def test_username_case_insensitive_matching(self): + chars = ['', 'İ', 'É', 'Ø', 'œ', 'ß', 'ss', 'Ξ', 'ф', '大家好'] + usernames = ['UsEr%s' % c for c in chars] + users = [ + UserFactory.create(username=username) for username in usernames] + for user in users: + data = { + 'username': user.username.lower(), + 'email': '%s@beatles.uk' % user.username, + 'password1': 'iloveyoko79!', + 'password2': 'iloveyoko79!', + 'full_name': 'John Lennon', + } + form = forms.RegisterForm(data) + assert form.is_valid() is False + assert (_("A user with that username already exists") in + form.errors.get('username')) + + user = UserFactory.create(username='JohNlEnNoN') + data = { + 'username': 'johnLennon', + 'email': 'john@beatles.uk', + 'password1': 'iloveyoko79!', + 'password2': 'iloveyoko79!', + 'full_name': 'John Lennon', + } + form = forms.RegisterForm(data) + assert form.is_valid() is False + assert (_("A user with that username already exists") in + form.errors.get('username')) + def test_passwords_do_not_match(self): data = { 'username': 'imagine71', @@ -241,6 +271,39 @@ def test_update_user_with_existing_username(self): form = forms.ProfileForm(data, instance=user) assert form.is_valid() is False + def test_username_case_insensitive_matching(self): + existing_user = UserFactory.create(username='TestUser') + chars = ['', 'İ', 'É', 'Ø', 'œ', 'ß', 'ss', 'Ξ', 'ф', '大家好'] + usernames = ['UsEr%s' % c for c in chars] + users = [ + UserFactory.create(username=username) for username in usernames] + for user in users: + data = { + 'username': user.username.lower(), + 'email': '%s@beatles.uk' % user.username, + 'full_name': 'John Lennon', + } + form = forms.ProfileForm(data, instance=existing_user) + assert form.is_valid() is False + assert (_("A user with that username already exists") in + form.errors.get('username')) + existing_user.refresh_from_db() + assert existing_user.username == 'TestUser' + + user = UserFactory.create( + username='JohNlEnNoN', email='john@beatles.uk') + data = { + 'username': 'johnLennon', + 'email': 'john@beatles.uk', + 'full_name': 'John Lennon', + } + form = forms.ProfileForm(data, instance=user) + assert form.is_valid() is True + form.save() + + user.refresh_from_db() + assert user.username == 'johnLennon' + def test_update_user_with_existing_email(self): UserFactory.create(email='existing@example.com') user = UserFactory.create(username='imagine71', diff --git a/cadasta/accounts/tests/test_serializers.py b/cadasta/accounts/tests/test_serializers.py index 56fadc87c..93c24668d 100644 --- a/cadasta/accounts/tests/test_serializers.py +++ b/cadasta/accounts/tests/test_serializers.py @@ -41,6 +41,24 @@ def test_create_with_valid_data(self): assert user_obj.is_active assert not user_obj.email_verified + def test_case_insensitive_username(self): + usernames = ['UsErOne', 'useRtWo', 'uSERthReE'] + users = [ + UserFactory.create(username=username) for username in usernames] + for user in users: + TEST_DATA = BASIC_TEST_DATA.copy() + TEST_DATA['username'] = user.username.lower() + serializer = serializers.RegistrationSerializer(data=TEST_DATA) + assert serializer.is_valid() is False + assert (_("A user with that username already exists") + in serializer._errors['username']) + TEST_DATA = BASIC_TEST_DATA.copy() + TEST_DATA['username'] = 'userFour' + serializer = serializers.RegistrationSerializer(data=TEST_DATA) + assert serializer.is_valid() + serializer.save() + assert User.objects.count() == 4 + def test_create_without_email(self): """Serialiser should be invalid when no email address is provided.""" @@ -208,6 +226,26 @@ def test_update_username_fails(self): assert not serializer2.is_valid() assert serializer2.errors['username'] == ['Cannot update username'] + def test_case_insensitive_username(self): + data = { + 'username': 'USERtwO', + 'email': 'john@beatles.uk', + 'password': 'iloveyoko79', + 'full_name': 'John Lennon' + } + usernames = ['UsErOne', 'useRtWo', 'uSERthReE'] + users = [ + UserFactory.create(username=username) for username in usernames] + request = APIRequestFactory().patch( + '/user/userone', data) + force_authenticate(request, user=users[0]) + serializer = serializers.UserSerializer( + users[0], data, context={'request': Request(request)}) + assert serializer.is_valid() is False + assert serializer.errors['username'] == [ + _("A user with that username already exists") + ] + def test_update_last_login_fails(self): serializer = serializers.UserSerializer(data=BASIC_TEST_DATA) assert serializer.is_valid() diff --git a/cadasta/accounts/validators.py b/cadasta/accounts/validators.py index dd2bf945d..fc49640c4 100644 --- a/cadasta/accounts/validators.py +++ b/cadasta/accounts/validators.py @@ -4,11 +4,11 @@ DEFAULT_CHARACTER_TYPES = [ - string.ascii_lowercase, - string.ascii_uppercase, - string.punctuation, - string.digits - ] + string.ascii_lowercase, + string.ascii_uppercase, + string.punctuation, + string.digits +] class CharacterTypePasswordValidator(object):