Fixed #19758 -- Avoided leaking email existence through the password reset form.
This commit is contained in:
parent
7acabbb980
commit
2f4a4703e1
|
@ -14,6 +14,8 @@
|
||||||
|
|
||||||
<h1>{% trans 'Password reset successful' %}</h1>
|
<h1>{% trans 'Password reset successful' %}</h1>
|
||||||
|
|
||||||
<p>{% trans "We've emailed you instructions for setting your password to the email address you submitted. You should be receiving it shortly." %}</p>
|
<p>{% trans "We've emailed you instructions for setting your password. You should be receiving them shortly." %}</p>
|
||||||
|
|
||||||
|
<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>
|
||||||
|
|
||||||
{% endblock %}
|
{% endblock %}
|
||||||
|
|
|
@ -206,31 +206,8 @@ class AuthenticationForm(forms.Form):
|
||||||
|
|
||||||
|
|
||||||
class PasswordResetForm(forms.Form):
|
class PasswordResetForm(forms.Form):
|
||||||
error_messages = {
|
|
||||||
'unknown': _("That email address doesn't have an associated "
|
|
||||||
"user account. Are you sure you've registered?"),
|
|
||||||
'unusable': _("The user account associated with this email "
|
|
||||||
"address cannot reset the password."),
|
|
||||||
}
|
|
||||||
email = forms.EmailField(label=_("Email"), max_length=254)
|
email = forms.EmailField(label=_("Email"), max_length=254)
|
||||||
|
|
||||||
def clean_email(self):
|
|
||||||
"""
|
|
||||||
Validates that an active user exists with the given email address.
|
|
||||||
"""
|
|
||||||
UserModel = get_user_model()
|
|
||||||
email = self.cleaned_data["email"]
|
|
||||||
self.users_cache = UserModel._default_manager.filter(email__iexact=email)
|
|
||||||
if not len(self.users_cache):
|
|
||||||
raise forms.ValidationError(self.error_messages['unknown'])
|
|
||||||
if not any(user.is_active for user in self.users_cache):
|
|
||||||
# none of the filtered users are active
|
|
||||||
raise forms.ValidationError(self.error_messages['unknown'])
|
|
||||||
if any((user.password == UNUSABLE_PASSWORD)
|
|
||||||
for user in self.users_cache):
|
|
||||||
raise forms.ValidationError(self.error_messages['unusable'])
|
|
||||||
return email
|
|
||||||
|
|
||||||
def save(self, domain_override=None,
|
def save(self, domain_override=None,
|
||||||
subject_template_name='registration/password_reset_subject.txt',
|
subject_template_name='registration/password_reset_subject.txt',
|
||||||
email_template_name='registration/password_reset_email.html',
|
email_template_name='registration/password_reset_email.html',
|
||||||
|
@ -241,7 +218,14 @@ class PasswordResetForm(forms.Form):
|
||||||
user.
|
user.
|
||||||
"""
|
"""
|
||||||
from django.core.mail import send_mail
|
from django.core.mail import send_mail
|
||||||
for user in self.users_cache:
|
UserModel = get_user_model()
|
||||||
|
email = self.cleaned_data["email"]
|
||||||
|
users = UserModel._default_manager.filter(email__iexact=email)
|
||||||
|
for user in users:
|
||||||
|
# Make sure that no email is sent to a user that actually has
|
||||||
|
# a password marked as unusable
|
||||||
|
if user.password == UNUSABLE_PASSWORD:
|
||||||
|
continue
|
||||||
if not domain_override:
|
if not domain_override:
|
||||||
current_site = get_current_site(request)
|
current_site = get_current_site(request)
|
||||||
site_name = current_site.name
|
site_name = current_site.name
|
||||||
|
|
|
@ -326,20 +326,28 @@ class PasswordResetFormTest(TestCase):
|
||||||
[force_text(EmailField.default_error_messages['invalid'])])
|
[force_text(EmailField.default_error_messages['invalid'])])
|
||||||
|
|
||||||
def test_nonexistant_email(self):
|
def test_nonexistant_email(self):
|
||||||
# Test nonexistant email address
|
# Test nonexistant email address. This should not fail because it would
|
||||||
|
# expose information about registered users.
|
||||||
data = {'email': 'foo@bar.com'}
|
data = {'email': 'foo@bar.com'}
|
||||||
form = PasswordResetForm(data)
|
form = PasswordResetForm(data)
|
||||||
self.assertFalse(form.is_valid())
|
self.assertTrue(form.is_valid())
|
||||||
self.assertEqual(form.errors,
|
self.assertEquals(len(mail.outbox), 0)
|
||||||
{'email': [force_text(form.error_messages['unknown'])]})
|
|
||||||
|
|
||||||
|
@override_settings(
|
||||||
|
TEMPLATE_LOADERS=('django.template.loaders.filesystem.Loader',),
|
||||||
|
TEMPLATE_DIRS=(
|
||||||
|
os.path.join(os.path.dirname(upath(__file__)), 'templates'),
|
||||||
|
),
|
||||||
|
)
|
||||||
def test_cleaned_data(self):
|
def test_cleaned_data(self):
|
||||||
# Regression test
|
# Regression test
|
||||||
(user, username, email) = self.create_dummy_user()
|
(user, username, email) = self.create_dummy_user()
|
||||||
data = {'email': email}
|
data = {'email': email}
|
||||||
form = PasswordResetForm(data)
|
form = PasswordResetForm(data)
|
||||||
self.assertTrue(form.is_valid())
|
self.assertTrue(form.is_valid())
|
||||||
|
form.save(domain_override='example.com')
|
||||||
self.assertEqual(form.cleaned_data['email'], email)
|
self.assertEqual(form.cleaned_data['email'], email)
|
||||||
|
self.assertEqual(len(mail.outbox), 1)
|
||||||
|
|
||||||
@override_settings(
|
@override_settings(
|
||||||
TEMPLATE_LOADERS=('django.template.loaders.filesystem.Loader',),
|
TEMPLATE_LOADERS=('django.template.loaders.filesystem.Loader',),
|
||||||
|
@ -373,7 +381,8 @@ class PasswordResetFormTest(TestCase):
|
||||||
user.is_active = False
|
user.is_active = False
|
||||||
user.save()
|
user.save()
|
||||||
form = PasswordResetForm({'email': email})
|
form = PasswordResetForm({'email': email})
|
||||||
self.assertFalse(form.is_valid())
|
self.assertTrue(form.is_valid())
|
||||||
|
self.assertEqual(len(mail.outbox), 0)
|
||||||
|
|
||||||
def test_unusable_password(self):
|
def test_unusable_password(self):
|
||||||
user = User.objects.create_user('testuser', 'test@example.com', 'test')
|
user = User.objects.create_user('testuser', 'test@example.com', 'test')
|
||||||
|
@ -383,9 +392,10 @@ class PasswordResetFormTest(TestCase):
|
||||||
user.set_unusable_password()
|
user.set_unusable_password()
|
||||||
user.save()
|
user.save()
|
||||||
form = PasswordResetForm(data)
|
form = PasswordResetForm(data)
|
||||||
self.assertFalse(form.is_valid())
|
# The form itself is valid, but no email is sent
|
||||||
self.assertEqual(form["email"].errors,
|
self.assertTrue(form.is_valid())
|
||||||
[_("The user account associated with this email address cannot reset the password.")])
|
form.save()
|
||||||
|
self.assertEquals(len(mail.outbox), 0)
|
||||||
|
|
||||||
|
|
||||||
class ReadOnlyPasswordHashTest(TestCase):
|
class ReadOnlyPasswordHashTest(TestCase):
|
||||||
|
|
|
@ -86,11 +86,12 @@ class AuthViewNamedURLTests(AuthViewsTestCase):
|
||||||
class PasswordResetTest(AuthViewsTestCase):
|
class PasswordResetTest(AuthViewsTestCase):
|
||||||
|
|
||||||
def test_email_not_found(self):
|
def test_email_not_found(self):
|
||||||
"Error is raised if the provided email address isn't currently registered"
|
"""If the provided email is not registered, don't raise any error but
|
||||||
|
also don't send any email."""
|
||||||
response = self.client.get('/password_reset/')
|
response = self.client.get('/password_reset/')
|
||||||
self.assertEqual(response.status_code, 200)
|
self.assertEqual(response.status_code, 200)
|
||||||
response = self.client.post('/password_reset/', {'email': 'not_a_real_email@email.com'})
|
response = self.client.post('/password_reset/', {'email': 'not_a_real_email@email.com'})
|
||||||
self.assertFormError(response, PasswordResetForm.error_messages['unknown'])
|
self.assertEqual(response.status_code, 302)
|
||||||
self.assertEqual(len(mail.outbox), 0)
|
self.assertEqual(len(mail.outbox), 0)
|
||||||
|
|
||||||
def test_email_found(self):
|
def test_email_found(self):
|
||||||
|
|
|
@ -743,10 +743,24 @@ patterns.
|
||||||
that can be used to reset the password, and sending that link to the
|
that can be used to reset the password, and sending that link to the
|
||||||
user's registered email address.
|
user's registered email address.
|
||||||
|
|
||||||
|
If the email address provided does not exist in the system, this view
|
||||||
|
won't send an email, but the user won't receive any error message either.
|
||||||
|
This prevents information leaking to potential attackers. If you want to
|
||||||
|
provide an error message in this case, you can subclass
|
||||||
|
:class:`~django.contrib.auth.forms.PasswordResetForm` and use the
|
||||||
|
``password_reset_form`` argument.
|
||||||
|
|
||||||
|
|
||||||
Users flagged with an unusable password (see
|
Users flagged with an unusable password (see
|
||||||
:meth:`~django.contrib.auth.models.User.set_unusable_password()` aren't
|
:meth:`~django.contrib.auth.models.User.set_unusable_password()` aren't
|
||||||
allowed to request a password reset to prevent misuse when using an
|
allowed to request a password reset to prevent misuse when using an
|
||||||
external authentication source like LDAP.
|
external authentication source like LDAP. Note that they won't receive any
|
||||||
|
error message since this would expose their account's existence but no
|
||||||
|
mail will be sent either.
|
||||||
|
|
||||||
|
.. versionchanged:: 1.6
|
||||||
|
Previously, error messages indicated whether a given email was
|
||||||
|
registered.
|
||||||
|
|
||||||
**URL name:** ``password_reset``
|
**URL name:** ``password_reset``
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue