From 0362b0e986303858081f607ffad2e8e14be8775e Mon Sep 17 00:00:00 2001 From: Jacob Walls <38668450+jacobtylerwalls@users.noreply.github.com> Date: Sun, 15 May 2016 18:54:03 -0700 Subject: [PATCH] Fixed #26615 -- Made password reset token invalidate when changing email. Co-Authored-By: Silas Barta --- django/contrib/auth/tokens.py | 10 ++++---- docs/releases/3.2.txt | 3 +++ tests/auth_tests/models/__init__.py | 9 ++++---- .../models/with_custom_email_field.py | 2 +- tests/auth_tests/test_models.py | 3 +-- tests/auth_tests/test_tokens.py | 23 +++++++++++++++++++ 6 files changed, 39 insertions(+), 11 deletions(-) diff --git a/django/contrib/auth/tokens.py b/django/contrib/auth/tokens.py index 93e2d71af2..c534f304f3 100644 --- a/django/contrib/auth/tokens.py +++ b/django/contrib/auth/tokens.py @@ -78,9 +78,9 @@ class PasswordResetTokenGenerator: def _make_hash_value(self, user, timestamp): """ - Hash the user's primary key and some user state that's sure to change - after a password reset to produce a token that invalidated when it's - used: + Hash the user's primary key, email (if available), and some user state + that's sure to change after a password reset to produce a token that is + invalidated when it's used: 1. The password field will change upon a password reset (even if the same password is chosen, due to password salting). 2. The last_login field will usually be updated very shortly after @@ -94,7 +94,9 @@ class PasswordResetTokenGenerator: # Truncate microseconds so that tokens are consistent even if the # database doesn't support microseconds. login_timestamp = '' if user.last_login is None else user.last_login.replace(microsecond=0, tzinfo=None) - return str(user.pk) + user.password + str(login_timestamp) + str(timestamp) + email_field = user.get_email_field_name() + email = getattr(user, email_field, '') or '' + return f'{user.pk}{user.password}{login_timestamp}{timestamp}{email}' def _num_seconds(self, dt): return int((dt - datetime(2001, 1, 1)).total_seconds()) diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt index 10842af4fc..232b20cb23 100644 --- a/docs/releases/3.2.txt +++ b/docs/releases/3.2.txt @@ -552,6 +552,9 @@ Miscellaneous ``False`` if the file cannot be locked, instead of raising :exc:`BlockingIOError`. +* The password reset mechanism now invalidates tokens when the user email is + changed. + .. _deprecated-features-3.2: Features deprecated in 3.2 diff --git a/tests/auth_tests/models/__init__.py b/tests/auth_tests/models/__init__.py index 003d8eeaa7..b3e06cd7b8 100644 --- a/tests/auth_tests/models/__init__.py +++ b/tests/auth_tests/models/__init__.py @@ -8,6 +8,7 @@ from .minimal import MinimalUser from .no_password import NoPasswordUser from .proxy import Proxy, UserProxy from .uuid_pk import UUIDUser +from .with_custom_email_field import CustomEmailField from .with_foreign_key import CustomUserWithFK, Email from .with_integer_username import IntegerUsernameUser from .with_last_login_attr import UserWithDisabledLastLoginField @@ -16,10 +17,10 @@ from .with_many_to_many import ( ) __all__ = ( - 'CustomPermissionsUser', 'CustomUser', 'CustomUserNonUniqueUsername', - 'CustomUserWithFK', 'CustomUserWithM2M', 'CustomUserWithM2MThrough', - 'CustomUserWithoutIsActiveField', 'Email', 'ExtensionUser', - 'IntegerUsernameUser', 'IsActiveTestUser1', 'MinimalUser', + 'CustomEmailField', 'CustomPermissionsUser', 'CustomUser', + 'CustomUserNonUniqueUsername', 'CustomUserWithFK', 'CustomUserWithM2M', + 'CustomUserWithM2MThrough', 'CustomUserWithoutIsActiveField', 'Email', + 'ExtensionUser', 'IntegerUsernameUser', 'IsActiveTestUser1', 'MinimalUser', 'NoPasswordUser', 'Organization', 'Proxy', 'UUIDUser', 'UserProxy', 'UserWithDisabledLastLoginField', ) diff --git a/tests/auth_tests/models/with_custom_email_field.py b/tests/auth_tests/models/with_custom_email_field.py index 27b1f810f2..278cd137a4 100644 --- a/tests/auth_tests/models/with_custom_email_field.py +++ b/tests/auth_tests/models/with_custom_email_field.py @@ -15,7 +15,7 @@ class CustomEmailFieldUserManager(BaseUserManager): class CustomEmailField(AbstractBaseUser): username = models.CharField(max_length=255) password = models.CharField(max_length=255) - email_address = models.EmailField() + email_address = models.EmailField(null=True) is_active = models.BooleanField(default=True) EMAIL_FIELD = 'email_address' diff --git a/tests/auth_tests/test_models.py b/tests/auth_tests/test_models.py index 5a4c5cdecd..01f523fdf2 100644 --- a/tests/auth_tests/test_models.py +++ b/tests/auth_tests/test_models.py @@ -17,8 +17,7 @@ from django.test import ( SimpleTestCase, TestCase, TransactionTestCase, override_settings, ) -from .models import IntegerUsernameUser -from .models.with_custom_email_field import CustomEmailField +from .models import CustomEmailField, IntegerUsernameUser class NaturalKeysTestCase(TestCase): diff --git a/tests/auth_tests/test_tokens.py b/tests/auth_tests/test_tokens.py index bba435be84..42ac71148e 100644 --- a/tests/auth_tests/test_tokens.py +++ b/tests/auth_tests/test_tokens.py @@ -7,6 +7,8 @@ from django.test import TestCase from django.test.utils import ignore_warnings from django.utils.deprecation import RemovedInDjango40Warning +from .models import CustomEmailField + class MockedPasswordResetTokenGenerator(PasswordResetTokenGenerator): def __init__(self, now): @@ -37,6 +39,27 @@ class TokenGeneratorTest(TestCase): tk2 = p0.make_token(user_reload) self.assertEqual(tk1, tk2) + def test_token_with_different_email(self): + """Updating the user email address invalidates the token.""" + tests = [ + (CustomEmailField, None), + (CustomEmailField, 'test4@example.com'), + (User, 'test4@example.com'), + ] + for model, email in tests: + with self.subTest(model=model.__qualname__, email=email): + user = model.objects.create_user( + 'changeemailuser', + email=email, + password='testpw', + ) + p0 = PasswordResetTokenGenerator() + tk1 = p0.make_token(user) + self.assertIs(p0.check_token(user, tk1), True) + setattr(user, user.get_email_field_name(), 'test4new@example.com') + user.save() + self.assertIs(p0.check_token(user, tk1), False) + def test_timeout(self): """The token is valid after n seconds, but no greater.""" # Uses a mocked version of PasswordResetTokenGenerator so we can change