#1497: Fix bad normalisation on Account.email

src/core/models.py

@@ -17,7 +17,7 @@
 from django.conf import settings
 from django.contrib.auth.models import AbstractBaseUser, BaseUserManager, PermissionsMixin
 from django.core.exceptions import ValidationError
-from django.db import models
+from django.db import models, transaction
 from django.utils import timezone
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.contrib.contenttypes.models import ContentType
@@ -146,16 +146,24 @@ def __str__(self):
         return self.name
 
 
+class AccountQuerySet(models.query.QuerySet):
+    def create(self, *args, **kwargs):
+        # Ensure cleaned fields on create and get_or_create
+        with transaction.atomic():
+            obj = super().create(*args, **kwargs)
+            obj.clean()
+            obj.save()
+            return obj
+
+
 class AccountManager(BaseUserManager):
     def create_user(self, email, password=None, **kwargs):
         if not email:
             raise ValueError('Users must have a valid email address.')
 
-        if not kwargs.get('username', None):
-            raise ValueError('Users must have a valid username.')
-
         account = self.model(
-            email=self.normalize_email(email), username=kwargs.get('username')
+            email=self.normalize_email(email),
+            username=email.lower(),
         )
 
         account.set_password(password)
@@ -174,6 +182,9 @@ def create_superuser(self, email, password, **kwargs):
 
         return account
 
+    def get_queryset(self):
+        return AccountQuerySet(self.model)
+
 
 class Account(AbstractBaseUser, PermissionsMixin):
     email = models.EmailField(unique=True, verbose_name=_('Email'))
@@ -223,11 +234,22 @@ class Account(AbstractBaseUser, PermissionsMixin):
 
     class Meta:
         ordering = ('first_name', 'last_name', 'username')
+        unique_together = ('email', 'username')
 
-    def save(self, *args, **kwargs):
+
+    def clean(self, *args, **kwargs):
+        """ Normalizes the email address
+
+        The username is lowercased instead, to cope with a bug present for
+        accounts imported/registered prior to v.1.3.8
+        https://github.com/BirkbeckCTP/janeway/issues/1497
+        The username being unique and lowercase at clean time avoids
+        the creation of duplicate email addresses where the casing might
+        be different.
+        """
+        self.email = self.__class__.objects.normalize_email(self.email)
         self.username = self.email.lower()
-        self.email = self.email.lower()
-        super(Account, self).save(*args, **kwargs)
+        super().clean(*args, **kwargs)
 
     def __str__(self):
         return self.full_name()

src/core/tests/test_app.py

@@ -42,6 +42,30 @@ def test_create_user_form(self):
         except models.Account.DoesNotExist:
             self.fail('User account has not been saved.')
 
+    @override_settings(URL_CONFIG="domain")
+    def test_create_user_form_normalise_email(self):
+
+        data = {
+            'email': '[email protected]',
+            'is_active': True,
+            'password_1': 'this_is_a_password',
+            'password_2': 'this_is_a_password',
+            'salutation': 'Prof.',
+            'first_name': 'Martin',
+            'last_name': 'Eve',
+            'department': 'English & Humanities',
+            'institution': 'Birkbeck, University of London',
+            'country': 235,
+        }
+
+        self.client.force_login(self.admin_user)
+        response = self.client.post(reverse('core_add_user'), data)
+
+        try:
+            models.Account.objects.get(email='[email protected]')
+        except models.Account.DoesNotExist:
+            self.fail('User account has not been saved.')
+
     def setUp(self):
         self.press = helpers.create_press()
         self.press.save()

src/core/tests/test_models.py

@@ -0,0 +1,58 @@
+from django.db import IntegrityError
+from django.test import TestCase
+
+from core import models
+
+
+class TestAccount(TestCase):
+    def test_creation(self):
+        data = {
+            'email': '[email protected]',
+            'is_active': True,
+            'password': 'this_is_a_password',
+            'salutation': 'Prof.',
+            'first_name': 'Martin',
+            'last_name': 'Eve',
+            'department': 'English & Humanities',
+            'institution': 'Birkbeck, University of London',
+        }
+
+        models.Account.objects.create(**data)
+        try:
+            models.Account.objects.get(email='[email protected]')
+        except models.Account.DoesNotExist:
+            self.fail('User account has not been created.')
+
+    def test_username_normalised(self):
+        email = "[email protected]"
+        data = {
+            'email': email,
+            'is_active': True,
+            'password': 'this_is_a_password',
+            'salutation': 'Prof.',
+            'first_name': 'Martin',
+            'last_name': 'Eve',
+            'department': 'English & Humanities',
+            'institution': 'Birkbeck, University of London',
+        }
+        obj = models.Account.objects.create(**data)
+        self.assertEquals(obj.username, email.lower())
+
+    def test_email_normalised(self):
+        email = "[email protected]"
+        expected = "[email protected]"
+        data = {
+            'email': email,
+        }
+        obj = models.Account.objects.create(**data)
+        self.assertEquals(obj.email, expected)
+
+    def test_no_duplicates(self):
+        email_a = "[email protected]"
+        email_b = "[email protected]"
+        models.Account.objects.create(email=email_a)
+        with self.assertRaises(
+            IntegrityError,
+            msg="Managed to register account with duplicate email",
+        ):
+            models.Account.objects.create(email=email_b)