Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #19758 -- Avoided leaking email existence through the password …

…reset form.
  • Loading branch information...
commit 2f4a4703e1931fadf5ed81387b26cf84caf5bef9 1 parent 7acabbb
Horst Gutmann authored February 23, 2013 aaugustin committed February 23, 2013
4  django/contrib/admin/templates/registration/password_reset_done.html
@@ -14,6 +14,8 @@
14 14
 
15 15
 <h1>{% trans 'Password reset successful' %}</h1>
16 16
 
17  
-<p>{% trans "We've emailed you instructions for setting your password to the email address you submitted. You should be receiving it shortly." %}</p>
  17
+<p>{% trans "We've emailed you instructions for setting your password. You should be receiving them shortly." %}</p>
  18
+
  19
+<p>{% trans "If you don't receive an email, please make sure you've entered the address you registered with, and check your spam folder." %}</p>
18 20
 
19 21
 {% endblock %}
32  django/contrib/auth/forms.py
@@ -206,31 +206,8 @@ def get_user(self):
206 206
 
207 207
 
208 208
 class PasswordResetForm(forms.Form):
209  
-    error_messages = {
210  
-        'unknown': _("That email address doesn't have an associated "
211  
-                     "user account. Are you sure you've registered?"),
212  
-        'unusable': _("The user account associated with this email "
213  
-                      "address cannot reset the password."),
214  
-    }
215 209
     email = forms.EmailField(label=_("Email"), max_length=254)
216 210
 
217  
-    def clean_email(self):
218  
-        """
219  
-        Validates that an active user exists with the given email address.
220  
-        """
221  
-        UserModel = get_user_model()
222  
-        email = self.cleaned_data["email"]
223  
-        self.users_cache = UserModel._default_manager.filter(email__iexact=email)
224  
-        if not len(self.users_cache):
225  
-            raise forms.ValidationError(self.error_messages['unknown'])
226  
-        if not any(user.is_active for user in self.users_cache):
227  
-            # none of the filtered users are active
228  
-            raise forms.ValidationError(self.error_messages['unknown'])
229  
-        if any((user.password == UNUSABLE_PASSWORD)
230  
-               for user in self.users_cache):
231  
-            raise forms.ValidationError(self.error_messages['unusable'])
232  
-        return email
233  
-
234 211
     def save(self, domain_override=None,
235 212
              subject_template_name='registration/password_reset_subject.txt',
236 213
              email_template_name='registration/password_reset_email.html',
@@ -241,7 +218,14 @@ def save(self, domain_override=None,
241 218
         user.
242 219
         """
243 220
         from django.core.mail import send_mail
244  
-        for user in self.users_cache:
  221
+        UserModel = get_user_model()
  222
+        email = self.cleaned_data["email"]
  223
+        users = UserModel._default_manager.filter(email__iexact=email)
  224
+        for user in users:
  225
+            # Make sure that no email is sent to a user that actually has
  226
+            # a password marked as unusable
  227
+            if user.password == UNUSABLE_PASSWORD:
  228
+                continue
245 229
             if not domain_override:
246 230
                 current_site = get_current_site(request)
247 231
                 site_name = current_site.name
26  django/contrib/auth/tests/forms.py
@@ -326,20 +326,28 @@ def test_invalid_email(self):
326 326
                          [force_text(EmailField.default_error_messages['invalid'])])
327 327
 
328 328
     def test_nonexistant_email(self):
329  
-        # Test nonexistant email address
  329
+        # Test nonexistant email address. This should not fail because it would
  330
+        # expose information about registered users.
330 331
         data = {'email': 'foo@bar.com'}
331 332
         form = PasswordResetForm(data)
332  
-        self.assertFalse(form.is_valid())
333  
-        self.assertEqual(form.errors,
334  
-                         {'email': [force_text(form.error_messages['unknown'])]})
  333
+        self.assertTrue(form.is_valid())
  334
+        self.assertEquals(len(mail.outbox), 0)
335 335
 
  336
+    @override_settings(
  337
+        TEMPLATE_LOADERS=('django.template.loaders.filesystem.Loader',),
  338
+        TEMPLATE_DIRS=(
  339
+            os.path.join(os.path.dirname(upath(__file__)), 'templates'),
  340
+        ),
  341
+    )
336 342
     def test_cleaned_data(self):
337 343
         # Regression test
338 344
         (user, username, email) = self.create_dummy_user()
339 345
         data = {'email': email}
340 346
         form = PasswordResetForm(data)
341 347
         self.assertTrue(form.is_valid())
  348
+        form.save(domain_override='example.com')
342 349
         self.assertEqual(form.cleaned_data['email'], email)
  350
+        self.assertEqual(len(mail.outbox), 1)
343 351
 
344 352
     @override_settings(
345 353
         TEMPLATE_LOADERS=('django.template.loaders.filesystem.Loader',),
@@ -373,7 +381,8 @@ def test_inactive_user(self):
373 381
         user.is_active = False
374 382
         user.save()
375 383
         form = PasswordResetForm({'email': email})
376  
-        self.assertFalse(form.is_valid())
  384
+        self.assertTrue(form.is_valid())
  385
+        self.assertEqual(len(mail.outbox), 0)
377 386
 
378 387
     def test_unusable_password(self):
379 388
         user = User.objects.create_user('testuser', 'test@example.com', 'test')
@@ -383,9 +392,10 @@ def test_unusable_password(self):
383 392
         user.set_unusable_password()
384 393
         user.save()
385 394
         form = PasswordResetForm(data)
386  
-        self.assertFalse(form.is_valid())
387  
-        self.assertEqual(form["email"].errors,
388  
-                         [_("The user account associated with this email address cannot reset the password.")])
  395
+        # The form itself is valid, but no email is sent
  396
+        self.assertTrue(form.is_valid())
  397
+        form.save()
  398
+        self.assertEquals(len(mail.outbox), 0)
389 399
 
390 400
 
391 401
 class ReadOnlyPasswordHashTest(TestCase):
5  django/contrib/auth/tests/views.py
@@ -86,11 +86,12 @@ def test_named_urls(self):
86 86
 class PasswordResetTest(AuthViewsTestCase):
87 87
 
88 88
     def test_email_not_found(self):
89  
-        "Error is raised if the provided email address isn't currently registered"
  89
+        """If the provided email is not registered, don't raise any error but
  90
+        also don't send any email."""
90 91
         response = self.client.get('/password_reset/')
91 92
         self.assertEqual(response.status_code, 200)
92 93
         response = self.client.post('/password_reset/', {'email': 'not_a_real_email@email.com'})
93  
-        self.assertFormError(response, PasswordResetForm.error_messages['unknown'])
  94
+        self.assertEqual(response.status_code, 302)
94 95
         self.assertEqual(len(mail.outbox), 0)
95 96
 
96 97
     def test_email_found(self):
16  docs/topics/auth/default.txt
@@ -743,10 +743,24 @@ patterns.
743 743
     that can be used to reset the password, and sending that link to the
744 744
     user's registered email address.
745 745
 
  746
+    If the email address provided does not exist in the system, this view
  747
+    won't send an email, but the user won't receive any error message either.
  748
+    This prevents information leaking to potential attackers. If you want to
  749
+    provide an error message in this case, you can subclass
  750
+    :class:`~django.contrib.auth.forms.PasswordResetForm` and use the
  751
+    ``password_reset_form`` argument.
  752
+
  753
+
746 754
     Users flagged with an unusable password (see
747 755
     :meth:`~django.contrib.auth.models.User.set_unusable_password()` aren't
748 756
     allowed to request a password reset to prevent misuse when using an
749  
-    external authentication source like LDAP.
  757
+    external authentication source like LDAP. Note that they won't receive any
  758
+    error message since this would expose their account's existence but no
  759
+    mail will be sent either.
  760
+
  761
+    .. versionchanged:: 1.6
  762
+        Previously, error messages indicated whether a given email was
  763
+        registered.
750 764
 
751 765
     **URL name:** ``password_reset``
752 766
 

0 notes on commit 2f4a470

Please sign in to comment.
Something went wrong with that request. Please try again.