Browse Source

Ignore capitalisation of local part of email address

Most email servers don't respect capitalisation, and many users don't
know about it. So Oscar now does what the rest of the world does, and
ignores the capitalisation when looking up an email address.

Fixes #1179.
master
Maik Hoepfel 11 years ago
parent
commit
860fab7ba7

+ 18
- 0
docs/source/releases/v0.8.rst View File

@@ -349,6 +349,24 @@ Other potentially breaking changes related to shipping include:
349 349
 * ``oscar.apps.shipping.Scales`` has been renamed and moved to
350 350
   ``oscar.apps.shipping.scales.Scale``, and is now overridable.
351 351
 
352
+Email address handling
353
+----------------------
354
+
355
+In theory, the local part of an email is case-sensitive. In practice, many
356
+users don't know about this and most email servers don't consider the
357
+capitalisation. Because of this, Oscar now disregards capitalisation when
358
+looking up emails (e.g. when a user logs in).
359
+Storing behaviour is unaltered: When a user's email address is stored (e.g.
360
+when registering or checking out), the local part is unaltered and
361
+the host portion is lowercased.
362
+
363
+.. warning::
364
+
365
+   Those changes mean you might now have multiple users with email addresses
366
+   that Oscar considers identical. Please use the new
367
+   ``oscar_find_duplicate_emails`` management command to check your database
368
+   and deal with any conflicts accordingly.
369
+
352 370
 Misc
353 371
 ~~~~
354 372
 

+ 1
- 1
oscar/apps/checkout/forms.py View File

@@ -56,7 +56,7 @@ class GatewayForm(AuthenticationForm):
56 56
                 del self.errors['password']
57 57
             if 'username' in self.cleaned_data:
58 58
                 email = normalise_email(self.cleaned_data['username'])
59
-                if User._default_manager.filter(email=email).exists():
59
+                if User._default_manager.filter(email__iexact=email).exists():
60 60
                     msg = "A user with that email address already exists"
61 61
                     self._errors["username"] = self.error_class([msg])
62 62
             return self.cleaned_data

+ 2
- 1
oscar/apps/customer/auth_backends.py View File

@@ -37,7 +37,8 @@ class EmailBackend(ModelBackend):
37 37
         # intentionally allow multiple users with the same email address
38 38
         # (has been a requirement in larger system deployments),
39 39
         # we just enforce that they don't share the same password.
40
-        matching_users = User.objects.filter(email=clean_email)
40
+        # We make a case-insensitive match when looking for emails.
41
+        matching_users = User.objects.filter(email__iexact=clean_email)
41 42
         authenticated_users = [
42 43
             user for user in matching_users if user.check_password(password)]
43 44
         if len(authenticated_users) == 1:

+ 11
- 5
oscar/apps/customer/forms.py View File

@@ -153,8 +153,11 @@ class EmailUserCreationForm(forms.ModelForm):
153 153
         super(EmailUserCreationForm, self).__init__(*args, **kwargs)
154 154
 
155 155
     def clean_email(self):
156
+        """
157
+        Checks for existing users with the supplied email address.
158
+        """
156 159
         email = normalise_email(self.cleaned_data['email'])
157
-        if User._default_manager.filter(email=email).exists():
160
+        if User._default_manager.filter(email__iexact=email).exists():
158 161
             raise forms.ValidationError(
159 162
                 _("A user with that email address already exists"))
160 163
         return email
@@ -280,9 +283,10 @@ class UserForm(forms.ModelForm):
280 283
         """
281 284
         email = normalise_email(self.cleaned_data['email'])
282 285
         if User._default_manager.filter(
283
-                email=email).exclude(id=self.user.id).exists():
286
+                email__iexact=email).exclude(id=self.user.id).exists():
284 287
             raise ValidationError(
285 288
                 _("A user with this email address already exists"))
289
+        # Save the email unaltered
286 290
         return email
287 291
 
288 292
     class Meta:
@@ -340,8 +344,10 @@ if Profile:
340 344
 
341 345
         def clean_email(self):
342 346
             email = normalise_email(self.cleaned_data['email'])
343
-            if User._default_manager.filter(
344
-                    email=email).exclude(id=self.instance.user.id).exists():
347
+
348
+            users_with_email = User._default_manager.filter(
349
+                email__iexact=email).exclude(id=self.instance.user.id)
350
+            if users_with_email.exists():
345 351
                 raise ValidationError(
346 352
                     _("A user with this email address already exists"))
347 353
             return email
@@ -392,7 +398,7 @@ class ProductAlertForm(forms.ModelForm):
392 398
         if email:
393 399
             try:
394 400
                 ProductAlert.objects.get(
395
-                    product=self.product, email=email,
401
+                    product=self.product, email__iexact=email,
396 402
                     status=ProductAlert.ACTIVE)
397 403
             except ProductAlert.DoesNotExist:
398 404
                 pass

+ 4
- 0
oscar/apps/customer/mixins.py View File

@@ -68,6 +68,10 @@ class RegisterUserMixin(object):
68 68
             logger.warning(
69 69
                 'Multiple users with identical email address and password'
70 70
                 'were found. Marking all but one as not active.')
71
+            # As this section explicitly deals with the form being submitted
72
+            # twice, this is about the only place in Oscar where we don't
73
+            # ignore capitalisation when looking up an email address.
74
+            # We might otherwise accidentally mark unrelated users as inactive
71 75
             users = User.objects.filter(email=user.email)
72 76
             user = users[0]
73 77
             for u in users[1:]:

+ 1
- 1
oscar/apps/dashboard/users/views.py View File

@@ -51,7 +51,7 @@ class IndexView(BulkEditMixin, ListView):
51 51
 
52 52
         if data['email']:
53 53
             email = normalise_email(data['email'])
54
-            queryset = queryset.filter(email__startswith=email)
54
+            queryset = queryset.filter(email__istartswith=email)
55 55
             self.desc_ctx['email_filter'] \
56 56
                 = _(" with email matching '%s'") % email
57 57
         if data['name']:

+ 3
- 6
sites/sandbox/apps/gateway/forms.py View File

@@ -1,17 +1,14 @@
1 1
 from django import forms
2 2
 from django.contrib.auth.models import User
3
+from apps.customer.utils import normalise_email
3 4
 
4 5
 
5 6
 class GatewayForm(forms.Form):
6 7
     email = forms.EmailField()
7 8
 
8 9
     def clean_email(self):
9
-        email = self.cleaned_data['email']
10
-        try:
11
-            User.objects.get(email=email)
12
-        except User.DoesNotExist:
13
-            pass
14
-        else:
10
+        email = normalise_email(self.cleaned_data['email'])
11
+        if User.objects.filter(email__iexact=email).exists():
15 12
             raise forms.ValidationError(
16 13
                 "A user already exists with email %s" % email
17 14
             )

+ 11
- 2
tests/functional/customer/auth_tests.py View File

@@ -93,6 +93,7 @@ class TestAnAuthenticatedUser(WebTestCase):
93 93
 
94 94
 
95 95
 class TestAnAnonymousUser(WebTestCase):
96
+    is_anonymous = True
96 97
 
97 98
     def assertCanLogin(self, email, password):
98 99
         url = reverse('customer:login')
@@ -121,11 +122,19 @@ class TestAnAnonymousUser(WebTestCase):
121 122
         url = reverse('customer:register')
122 123
         form = self.app.get(url).forms['register_form']
123 124
         form['email'] = 'terry@boom.com'
124
-        form['password1'] = 'hedgehog'
125
-        form['password2'] = 'hedgehog'
125
+        form['password1'] = form['password2'] = 'hedgehog'
126 126
         response = form.submit()
127 127
         self.assertRedirectsTo(response, 'customer:summary')
128 128
 
129
+    def test_casing_of_local_part_of_email_is_preserved(self):
130
+        url = reverse('customer:register')
131
+        form = self.app.get(url).forms['register_form']
132
+        form['email'] = 'Terry@Boom.com'
133
+        form['password1'] = form['password2'] = 'hedgehog'
134
+        form.submit()
135
+        user = User.objects.all()[0]
136
+        self.assertEquals(user.email, 'Terry@boom.com')
137
+
129 138
 
130 139
 class TestAStaffUser(WebTestCase):
131 140
     is_anonymous = True

+ 20
- 21
tests/functional/customer/profile_tests.py View File

@@ -76,29 +76,28 @@ class TestASignedInUser(WebTestCase):
76 76
         self.assertEqual('Chuckle', user.last_name)
77 77
 
78 78
     def test_cant_update_their_email_address_if_it_already_exists(self):
79
-        User.objects.create_user(username='testuser', email='new@example.com',
80
-                                 password="somerandompassword")
79
+        # create a user to "block" new@example.com
80
+        User.objects.create_user(
81
+            username='testuser', email='new@example.com',
82
+            password="somerandompassword")
81 83
         self.assertEqual(User.objects.count(), 2)
82 84
 
83
-        profile_form_page = self.app.get(reverse('customer:profile-update'),
84
-                                user=self.user)
85
-        self.assertEqual(200, profile_form_page.status_code)
86
-        form = profile_form_page.forms['profile_form']
87
-        form['email'] = 'new@example.com'
88
-        form['first_name'] = 'Barry'
89
-        form['last_name'] = 'Chuckle'
90
-        response = form.submit()
91
-
92
-        user = User.objects.get(id=self.user.id)
93
-        self.assertEqual(self.email, user.email)
94
-
95
-        try:
96
-            User.objects.get(email='new@example.com')
97
-        except User.MultipleObjectsReturned:
98
-            self.fail("email for user changed to existing one")
99
-
100
-        self.assertContains(response,
101
-                            'A user with this email address already exists')
85
+        for email in ['new@example.com', 'New@Example.com']:
86
+            profile_form_page = self.app.get(
87
+                reverse('customer:profile-update'), user=self.user)
88
+            form = profile_form_page.forms['profile_form']
89
+            form['email'] = email
90
+            form['first_name'] = 'Barry'
91
+            form['last_name'] = 'Chuckle'
92
+            response = form.submit()
93
+
94
+            # assert that the original user's email address is unchanged
95
+            user = User.objects.get(id=self.user.id)
96
+            self.assertEqual(self.email, user.email)
97
+            self.assertEquals(
98
+                User.objects.filter(email__iexact='new@example.com').count(), 1)
99
+            self.assertContains(
100
+                response, 'A user with this email address already exists')
102 101
 
103 102
     def test_can_change_their_password(self):
104 103
         new_password = 'bubblesgopop'

+ 22
- 0
tests/integration/auth_tests.py View File

@@ -9,6 +9,28 @@ from oscar.core.compat import get_user_model
9 9
 User = get_user_model()
10 10
 
11 11
 
12
+class TestEmailAuthBackend(TestCase):
13
+
14
+    def test_authenticates_multiple_users(self):
15
+        password = 'lookmanohands'
16
+        users = [
17
+            User.objects.create_user(email, email, password=password)
18
+            for email in ['user1@example.com', 'user2@example.com']]
19
+        for created_user in users:
20
+            user = authenticate(username=created_user.email, password=password)
21
+            self.assertEquals(user, created_user)
22
+
23
+    def test_authenticates_different_email_spelling(self):
24
+        email = password = 'person@example.com'
25
+        created_user = User.objects.create_user(
26
+            'user1', email, password=password)
27
+
28
+        for email_variation in [
29
+            'Person@example.com', 'Person@EXAMPLE.COM', 'person@Example.com']:
30
+            user = authenticate(username=email_variation, password=password)
31
+            self.assertEquals(user, created_user)
32
+
33
+
12 34
 # Skip these tests for now as they only make sense when there isn't a unique
13 35
 # index on the user class.  The test suite currently uses a custom model that
14 36
 # *does* have a unique index on email.  When I figure out how to swap the user

Loading…
Cancel
Save