Merge pull request #8 from ixc/7-make-email-user-email-case-insensitive

Make email user email case insensitive

polymorphic_auth/admin.py

@@ -2,7 +2,8 @@
 from django.contrib import admin
 from django.contrib.auth.admin import UserAdmin
 from django.contrib.auth.forms import \
-    ReadOnlyPasswordHashField, UserChangeForm, UserCreationForm
+    ReadOnlyPasswordHashField, UserChangeForm as DjangoUserChangeForm, \
+    UserCreationForm
 from django.utils.translation import ugettext_lazy as _
 from polymorphic_auth.models import User
 from polymorphic.admin import \
@@ -54,6 +55,24 @@ def get_child_type_choices(self, request, action):
         return choices
 
 
+def _check_for_username_case_insensitive_clash(form):
+    """
+    Check for potential duplicate users before save for user types with
+    a case-insensitive `username` field.
+    """
+    user = form.instance
+    if user and user.IS_USERNAME_CASE_INSENSITIVE:
+        username = form.cleaned_data[user.USERNAME_FIELD]
+        matching_users = type(user).objects.filter(**{
+            '%s__iexact' % user.USERNAME_FIELD: username,
+        })
+        if user.pk:
+            matching_users = matching_users.exclude(pk=user.pk)
+        if matching_users:
+            raise forms.ValidationError(
+                u"A user with that %s already exists." % user.USERNAME_FIELD)
+
+
 def create_user_creation_form(user_model, user_model_fields):
     """
     Creates a creation form for the user model and model fields.
@@ -86,6 +105,10 @@ def clean_password2(self):
                 )
             return password2
 
+        def clean(self):
+            super(_UserCreationForm, self).clean()
+            _check_for_username_case_insensitive_clash(self)
+
         def save(self, commit=True):
             user = super(_UserCreationForm, self).save(commit=False)
             user.set_password(self.cleaned_data["password1"])
@@ -94,10 +117,15 @@ def save(self, commit=True):
             return user
 
     class CreationForm(UserCreationForm):
+
         class Meta:
             model = user_model
             fields = user_model_fields
 
+        def clean(self):
+            super(CreationForm, self).clean()
+            _check_for_username_case_insensitive_clash(self)
+
     if django_version < (1, 8):
         return _UserCreationForm
     return CreationForm
@@ -123,13 +151,24 @@ def __init__(self, *args, **kwargs):
         if f is not None:
             f.queryset = f.queryset.select_related('content_type')
 
+    def clean(self):
+        super(_UserChangeForm, self).clean()
+        _check_for_username_case_insensitive_clash(self)
+
     def clean_password(self):
         # Regardless of what the user provides, return the initial value.
         # This is done here, rather than on the field, because the
         # field does not have access to the initial value
         return self.initial["password"]
 
 
+class UserChangeForm(DjangoUserChangeForm):
+
+    def clean(self):
+        super(UserChangeForm, self).clean()
+        _check_for_username_case_insensitive_clash(self)
+
+
 class UserChildAdmin(PolymorphicChildModelAdmin):
     base_fieldsets = (
         ('Meta', {

polymorphic_auth/models.py

@@ -196,6 +196,18 @@ def create_superuser(self, password, **extra_fields):
         extra_fields.update(is_staff=True, is_superuser=True)
         return self._create_user(password, **extra_fields)
 
+    def get_by_natural_key(self, username):
+        """
+        Override default user lookup behaviour to match username (really email)
+        field with case INsensitivity for email-address based users.
+        """
+        if getattr(self.model, 'IS_USERNAME_CASE_INSENSITIVE', False):
+            return self.get(**{
+                '%s__iexact' % self.model.USERNAME_FIELD: username.lower()
+            })
+        else:
+            return super(UserManager, self).get_by_natural_key(username)
+
 
 # MODELS ######################################################################
 
@@ -211,6 +223,8 @@ class AbstractUser(PolymorphicModel, AbstractBaseUser):
 
     USERNAME_FIELD = 'id'
 
+    IS_USERNAME_CASE_INSENSITIVE = False
+
     class Meta:
         abstract = True
         verbose_name = _('user with ID login')
@@ -275,6 +289,23 @@ class AbstractAdminUser(
     class Meta:
         abstract = True
 
+    def save(self, *args, **kwargs):
+        # Hack to force check for potential duplicate users before save, in
+        # case more user-friendly validation sanity checks have not been
+        # implemented or have been bypassed.
+        if self.IS_USERNAME_CASE_INSENSITIVE:
+            matching_users = type(self).objects.filter(**{
+                '%s__iexact' % self.USERNAME_FIELD: self.username
+            })
+            if self.pk:
+                matching_users = matching_users.exclude(pk=self.pk)
+            if matching_users:
+                raise Exception(
+                    u"Identifier field %s='%s' matches existing users: %s"
+                    % (self.USERNAME_FIELD, self.username, matching_users))
+
+        super(AbstractAdminUser, self).save(*args, **kwargs)
+
 
 # Monkey-patch Django 1.7's `AbstractBaseUser` fields to match the field
 # settings as applied in Django 1.8, to make our `AbstractAdminUser` model

polymorphic_auth/usertypes/email/models.py

@@ -10,6 +10,8 @@ class AbstractEmailUser(User, EmailFieldMixin):
     USERNAME_FIELD = 'email'
     REQUIRED_FIELDS = ['first_name', 'last_name']
 
+    IS_USERNAME_CASE_INSENSITIVE = True
+
     class Meta:
         abstract = True
         verbose_name = _('user with email login')

polymorphic_auth/usertypes/email/tests.py

@@ -0,0 +1,131 @@
+from django_webtest import WebTest
+
+from django.core.urlresolvers import reverse
+
+from .models import EmailUser
+
+
+class TestEmailUser(WebTest):
+
+    def setUp(self):
+        self.superuser = EmailUser.objects.create(
+            email='superuser@test.com',
+            is_staff=True,
+            is_active=True,
+            is_superuser=True,
+        )
+        self.superuser.set_password('abc123')
+        self.superuser.save()
+
+    def test_cannot_create_user_with_same_email(self):
+        # Cannot create a user with an exactly matching email
+        try:
+            EmailUser.objects.create(email='superuser@test.com')
+        except Exception, ex:
+            self.assertTrue('matches existing users' in ex.message)
+
+        # Cannot create a user with an equivalent email when case is ignored
+        try:
+            EmailUser.objects.create(email='Superuser@test.com')
+        except Exception, ex:
+            self.assertTrue('matches existing users' in ex.message)
+
+    def test_cannot_modify_user_to_have_same_email(self):
+        user = EmailUser.objects.create(email='another@test.com')
+        # Cannot create a user with an exactly matching email
+        user.email = 'superuser@test.com'
+        try:
+            user.save()
+        except Exception, ex:
+            self.assertTrue('matches existing users' in ex.message)
+
+        # Cannot create a user with an equivalent email when case is ignored
+        user.email = 'Superuser@test.com'
+        try:
+            user.save()
+        except Exception, ex:
+            self.assertTrue('matches existing users' in ex.message)
+
+    def test_can_modify_existing_user(self):
+        self.superuser.first_name = 'Sir'
+        self.superuser.last_name = 'Test'
+        self.superuser.save()
+        reloaded_superuser = EmailUser.objects.get(pk=self.superuser.pk)
+        self.assertEqual('Sir', reloaded_superuser.first_name)
+        self.assertEqual('Test', reloaded_superuser.last_name)
+
+    def test_cannot_create_user_with_same_email_in_admin(self):
+        response = self.app.get(
+            reverse('admin:polymorphic_auth_user_add'),
+            user=self.superuser
+        ).maybe_follow()
+        form = response.form
+
+        # Cannot create a user with an exactly matching email
+        form['email'] = 'superuser@test.com'
+        form['password1'] = 'testpassword'
+        form['password2'] = 'testpassword'
+        response = form.submit(user=self.superuser)
+        self.assertTrue(
+            # Default form field error on 'email' field
+            'A user with that email address already exists' in response.text)
+        self.assertTrue(
+            # General error on "username" identifier field
+            'A user with that email already exists' in response.text)
+
+        # Cannot create a user with an equivalent email when case is ignored
+        form['email'] = 'Superuser@test.com'
+        form['password1'] = 'testpassword'
+        form['password2'] = 'testpassword'
+        response = form.submit(user=self.superuser)
+        self.assertFalse(
+            # Default form field error on 'email' field NOT PRESENT
+            'A user with that email address already exists' in response.text)
+        self.assertTrue(
+            # General error on "username" identifier field
+            'A user with that email already exists' in response.text)
+
+    def test_cannot_modify_user_to_have_same_email_in_admin(self):
+        user = EmailUser.objects.create(email='another@test.com')
+
+        response = self.app.get(
+            reverse('admin:polymorphic_auth_user_change',
+                    args=(user.pk,)),
+            user=self.superuser
+        ).maybe_follow()
+        form = response.form
+
+        # Cannot modify a user to have an exactly matching email
+        form['email'] = 'superuser@test.com'
+        response = form.submit(user=self.superuser)
+        self.assertTrue(
+            # Default form field error on 'email' field
+            'A user with that email address already exists' in response.text)
+        self.assertTrue(
+            # General error on "username" identifier field
+            'A user with that email already exists' in response.text)
+
+        # Cannot modify a user to have an equivalent email when case is ignored
+        form['email'] = 'Superuser@test.com'
+        response = form.submit(user=self.superuser)
+        self.assertFalse(
+            # Default form field error on 'email' field NOT PRESENT
+            'A user with that email address already exists' in response.text)
+        self.assertTrue(
+            # General error on "username" identifier field
+            'A user with that email already exists' in response.text)
+
+    def test_can_modify_existing_user_in_admin(self):
+        self.superuser.first_name = 'Sir Test'
+        response = self.app.get(
+            reverse('admin:polymorphic_auth_user_change',
+                    args=(self.superuser.pk,)),
+            user=self.superuser
+        ).maybe_follow()
+        form = response.form
+        form['first_name'] = 'Sir'
+        form['last_name'] = 'Test'
+        response = form.submit()
+        reloaded_superuser = EmailUser.objects.get(pk=self.superuser.pk)
+        self.assertEqual('Sir', reloaded_superuser.first_name)
+        self.assertEqual('Test', reloaded_superuser.last_name)