Fixed #28622 -- Allowed specifying password reset link expiration in seconds and deprecated PASSWORD_RESET_TIMEOUT_DAYS.

8 files changed, 178 insertions, 27 deletions

diff --git a/django/conf/__init__.py b/django/conf/__init__.py
index 5731fe912c..b32e56184d 100644
--- a/django/conf/__init__.py
+++ b/django/conf/__init__.py
@@ -9,14 +9,23 @@ for a list of all possible variables.
 import importlib
 import os
 import time
+import traceback
+import warnings
 from pathlib import Path
 
+import django
 from django.conf import global_settings
 from django.core.exceptions import ImproperlyConfigured
+from django.utils.deprecation import RemovedInDjango40Warning
 from django.utils.functional import LazyObject, empty
 
 ENVIRONMENT_VARIABLE = "DJANGO_SETTINGS_MODULE"
 
+PASSWORD_RESET_TIMEOUT_DAYS_DEPRECATED_MSG = (
+    'The PASSWORD_RESET_TIMEOUT_DAYS setting is deprecated. Use '
+    'PASSWORD_RESET_TIMEOUT instead.'
+)
+
 
 class SettingsReference(str):
     """
@@ -105,6 +114,20 @@ class LazySettings(LazyObject):
         """Return True if the settings have already been configured."""
         return self._wrapped is not empty
 
+    @property
+    def PASSWORD_RESET_TIMEOUT_DAYS(self):
+        stack = traceback.extract_stack()
+        # Show a warning if the setting is used outside of Django.
+        # Stack index: -1 this line, -2 the caller.
+        filename, _, _, _ = stack[-2]
+        if not filename.startswith(os.path.dirname(django.__file__)):
+            warnings.warn(
+                PASSWORD_RESET_TIMEOUT_DAYS_DEPRECATED_MSG,
+                RemovedInDjango40Warning,
+                stacklevel=2,
+            )
+        return self.__getattr__('PASSWORD_RESET_TIMEOUT_DAYS')
+
 
 class Settings:
     def __init__(self, settings_module):
@@ -137,6 +160,15 @@ class Settings:
         if not self.SECRET_KEY:
             raise ImproperlyConfigured("The SECRET_KEY setting must not be empty.")
 
+        if self.is_overridden('PASSWORD_RESET_TIMEOUT_DAYS'):
+            if self.is_overridden('PASSWORD_RESET_TIMEOUT'):
+                raise ImproperlyConfigured(
+                    'PASSWORD_RESET_TIMEOUT_DAYS/PASSWORD_RESET_TIMEOUT are '
+                    'mutually exclusive.'
+                )
+            setattr(self, 'PASSWORD_RESET_TIMEOUT', self.PASSWORD_RESET_TIMEOUT_DAYS * 60 * 60 * 24)
+            warnings.warn(PASSWORD_RESET_TIMEOUT_DAYS_DEPRECATED_MSG, RemovedInDjango40Warning)
+
         if hasattr(time, 'tzset') and self.TIME_ZONE:
             # When we can, attempt to validate the timezone. If we can't find
             # this file, no check happens and it's harmless.
@@ -180,6 +212,9 @@ class UserSettingsHolder:
 
     def __setattr__(self, name, value):
         self._deleted.discard(name)
+        if name == 'PASSWORD_RESET_TIMEOUT_DAYS':
+            setattr(self, 'PASSWORD_RESET_TIMEOUT', value * 60 * 60 * 24)
+            warnings.warn(PASSWORD_RESET_TIMEOUT_DAYS_DEPRECATED_MSG, RemovedInDjango40Warning)
         super().__setattr__(name, value)
 
     def __delattr__(self, name):
diff --git a/django/conf/global_settings.py b/django/conf/global_settings.py
index 398696530a..75935df0e1 100644
--- a/django/conf/global_settings.py
+++ b/django/conf/global_settings.py
@@ -506,6 +506,10 @@ LOGOUT_REDIRECT_URL = None
 # The number of days a password reset link is valid for
 PASSWORD_RESET_TIMEOUT_DAYS = 3
 
+# The minimum number of seconds a password reset link is valid for
+# (default: 3 days).
+PASSWORD_RESET_TIMEOUT = 60 * 60 * 24 * 3
+
 # the first hasher in this list is the preferred algorithm.  any
 # password using different algorithms will be converted automatically
 # upon login
diff --git a/django/contrib/auth/tokens.py b/django/contrib/auth/tokens.py
index 80fe699395..c4f2641003 100644
--- a/django/contrib/auth/tokens.py
+++ b/django/contrib/auth/tokens.py
@@ -1,4 +1,4 @@
-from datetime import date
+from datetime import datetime
 
 from django.conf import settings
 from django.utils.crypto import constant_time_compare, salted_hmac
@@ -18,7 +18,7 @@ class PasswordResetTokenGenerator:
         Return a token that can be used once to do a password reset
         for the given user.
         """
-        return self._make_token_with_timestamp(user, self._num_days(self._today()))
+        return self._make_token_with_timestamp(user, self._num_seconds(self._now()))
 
     def check_token(self, user, token):
         """
@@ -41,19 +41,15 @@ class PasswordResetTokenGenerator:
         if not constant_time_compare(self._make_token_with_timestamp(user, ts), token):
             return False
 
-        # Check the timestamp is within limit. Timestamps are rounded to
-        # midnight (server time) providing a resolution of only 1 day. If a
-        # link is generated 5 minutes before midnight and used 6 minutes later,
-        # that counts as 1 day. Therefore, PASSWORD_RESET_TIMEOUT_DAYS = 1 means
-        # "at least 1 day, could be up to 2."
-        if (self._num_days(self._today()) - ts) > settings.PASSWORD_RESET_TIMEOUT_DAYS:
+        # Check the timestamp is within limit.
+        if (self._num_seconds(self._now()) - ts) > settings.PASSWORD_RESET_TIMEOUT:
             return False
 
         return True
 
     def _make_token_with_timestamp(self, user, timestamp):
-        # timestamp is number of days since 2001-1-1.  Converted to
-        # base 36, this gives us a 3 digit string until about 2121
+        # timestamp is number of seconds since 2001-1-1. Converted to base 36,
+        # this gives us a 6 digit string until about 2069.
         ts_b36 = int_to_base36(timestamp)
         hash_string = salted_hmac(
             self.key_salt,
@@ -71,7 +67,7 @@ class PasswordResetTokenGenerator:
            same password is chosen, due to password salting).
         2. The last_login field will usually be updated very shortly after
            a password reset.
-        Failing those things, settings.PASSWORD_RESET_TIMEOUT_DAYS eventually
+        Failing those things, settings.PASSWORD_RESET_TIMEOUT eventually
         invalidates the token.
 
         Running this data through salted_hmac() prevents password cracking
@@ -82,12 +78,12 @@ class PasswordResetTokenGenerator:
         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)
 
-    def _num_days(self, dt):
-        return (dt - date(2001, 1, 1)).days
+    def _num_seconds(self, dt):
+        return int((dt - datetime(2001, 1, 1)).total_seconds())
 
-    def _today(self):
+    def _now(self):
         # Used for mocking in tests
-        return date.today()
+        return datetime.now()
 
 
 default_token_generator = PasswordResetTokenGenerator()
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index 03e9fa93ce..d79641a19c 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -34,6 +34,8 @@ details on these changes.
 
 * ``django.utils.http.is_safe_url()`` will be removed.
 
+* The ``PASSWORD_RESET_TIMEOUT_DAYS`` setting will be removed.
+
 See the :ref:`Django 3.1 release notes <deprecated-features-3.1>` for more
 details on these changes.
 
diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt
index a517a72f2f..fe103162ef 100644
--- a/docs/ref/settings.txt
+++ b/docs/ref/settings.txt
@@ -2872,6 +2872,19 @@ doesn't have a ``next_page`` attribute.
 If ``None``, no redirect will be performed and the logout view will be
 rendered.
 
+.. setting:: PASSWORD_RESET_TIMEOUT
+
+``PASSWORD_RESET_TIMEOUT``
+--------------------------
+
+.. versionadded:: 3.1
+
+Default: ``259200`` (3 days, in seconds)
+
+The minimum number of seconds a password reset link is valid for.
+
+Used by the :class:`~django.contrib.auth.views.PasswordResetConfirmView`.
+
 .. setting:: PASSWORD_RESET_TIMEOUT_DAYS
 
 ``PASSWORD_RESET_TIMEOUT_DAYS``
@@ -2884,6 +2897,10 @@ when the link is generated, it will be valid for up to a day longer.
 
 Used by the :class:`~django.contrib.auth.views.PasswordResetConfirmView`.
 
+.. deprecated:: 3.1
+
+    This setting is deprecated. Use :setting:`PASSWORD_RESET_TIMEOUT` instead.
+
 .. setting:: PASSWORD_HASHERS
 
 ``PASSWORD_HASHERS``
diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt
index 865ae60ec3..d448fc57b0 100644
--- a/docs/releases/3.1.txt
+++ b/docs/releases/3.1.txt
@@ -46,6 +46,11 @@ Minor features
 * The default iteration count for the PBKDF2 password hasher is increased from
   180,000 to 216,000.
 
+* Added the :setting:`PASSWORD_RESET_TIMEOUT` setting to define the minimum
+  number of seconds a password reset link is valid for. This is encouraged
+  instead of deprecated ``PASSWORD_RESET_TIMEOUT_DAYS``, which will be removed
+  in Django 4.0.
+
 :mod:`django.contrib.contenttypes`
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -222,7 +227,8 @@ Features deprecated in 3.1
 Miscellaneous
 -------------
 
-* ...
+* ``PASSWORD_RESET_TIMEOUT_DAYS`` setting is deprecated in favor of
+  :setting:`PASSWORD_RESET_TIMEOUT`.
 
 .. _removed-features-3.1:
 
diff --git a/tests/auth_tests/test_password_reset_timeout_days.py b/tests/auth_tests/test_password_reset_timeout_days.py
new file mode 100644
index 0000000000..db9aa62726
--- /dev/null
+++ b/tests/auth_tests/test_password_reset_timeout_days.py
@@ -0,0 +1,88 @@
+import sys
+from datetime import datetime, timedelta
+from types import ModuleType
+
+from django.conf import (
+    PASSWORD_RESET_TIMEOUT_DAYS_DEPRECATED_MSG, Settings, settings,
+)
+from django.contrib.auth.models import User
+from django.contrib.auth.tokens import PasswordResetTokenGenerator
+from django.core.exceptions import ImproperlyConfigured
+from django.test import TestCase, ignore_warnings
+from django.utils.deprecation import RemovedInDjango40Warning
+
+
+class DeprecationTests(TestCase):
+    msg = PASSWORD_RESET_TIMEOUT_DAYS_DEPRECATED_MSG
+
+    @ignore_warnings(category=RemovedInDjango40Warning)
+    def test_timeout(self):
+        """The token is valid after n days, but no greater."""
+        # Uses a mocked version of PasswordResetTokenGenerator so we can change
+        # the value of 'now'.
+        class Mocked(PasswordResetTokenGenerator):
+            def __init__(self, now):
+                self._now_val = now
+
+            def _now(self):
+                return self._now_val
+
+        user = User.objects.create_user('tokentestuser', 'test2@example.com', 'testpw')
+        p0 = PasswordResetTokenGenerator()
+        tk1 = p0.make_token(user)
+        p1 = Mocked(datetime.now() + timedelta(settings.PASSWORD_RESET_TIMEOUT_DAYS))
+        self.assertTrue(p1.check_token(user, tk1))
+        p2 = Mocked(datetime.now() + timedelta(settings.PASSWORD_RESET_TIMEOUT_DAYS + 1))
+        self.assertFalse(p2.check_token(user, tk1))
+        with self.settings(PASSWORD_RESET_TIMEOUT_DAYS=1):
+            self.assertEqual(settings.PASSWORD_RESET_TIMEOUT, 60 * 60 * 24)
+            p3 = Mocked(datetime.now() + timedelta(settings.PASSWORD_RESET_TIMEOUT_DAYS))
+            self.assertTrue(p3.check_token(user, tk1))
+            p4 = Mocked(datetime.now() + timedelta(settings.PASSWORD_RESET_TIMEOUT_DAYS + 1))
+            self.assertFalse(p4.check_token(user, tk1))
+
+    def test_override_settings_warning(self):
+        with self.assertRaisesMessage(RemovedInDjango40Warning, self.msg):
+            with self.settings(PASSWORD_RESET_TIMEOUT_DAYS=2):
+                pass
+
+    def test_settings_init_warning(self):
+        settings_module = ModuleType('fake_settings_module')
+        settings_module.SECRET_KEY = 'foo'
+        settings_module.PASSWORD_RESET_TIMEOUT_DAYS = 2
+        sys.modules['fake_settings_module'] = settings_module
+        try:
+            with self.assertRaisesMessage(RemovedInDjango40Warning, self.msg):
+                Settings('fake_settings_module')
+        finally:
+            del sys.modules['fake_settings_module']
+
+    def test_access_warning(self):
+        with self.assertRaisesMessage(RemovedInDjango40Warning, self.msg):
+            settings.PASSWORD_RESET_TIMEOUT_DAYS
+        # Works a second time.
+        with self.assertRaisesMessage(RemovedInDjango40Warning, self.msg):
+            settings.PASSWORD_RESET_TIMEOUT_DAYS
+
+    @ignore_warnings(category=RemovedInDjango40Warning)
+    def test_access(self):
+        with self.settings(PASSWORD_RESET_TIMEOUT_DAYS=2):
+            self.assertEqual(settings.PASSWORD_RESET_TIMEOUT_DAYS, 2)
+            # Works a second time.
+            self.assertEqual(settings.PASSWORD_RESET_TIMEOUT_DAYS, 2)
+
+    def test_use_both_settings_init_error(self):
+        msg = (
+            'PASSWORD_RESET_TIMEOUT_DAYS/PASSWORD_RESET_TIMEOUT are '
+            'mutually exclusive.'
+        )
+        settings_module = ModuleType('fake_settings_module')
+        settings_module.SECRET_KEY = 'foo'
+        settings_module.PASSWORD_RESET_TIMEOUT_DAYS = 2
+        settings_module.PASSWORD_RESET_TIMEOUT = 2000
+        sys.modules['fake_settings_module'] = settings_module
+        try:
+            with self.assertRaisesMessage(ImproperlyConfigured, msg):
+                Settings('fake_settings_module')
+        finally:
+            del sys.modules['fake_settings_module']
diff --git a/tests/auth_tests/test_tokens.py b/tests/auth_tests/test_tokens.py
index ede7b007fa..6d6caab9b1 100644
--- a/tests/auth_tests/test_tokens.py
+++ b/tests/auth_tests/test_tokens.py
@@ -1,4 +1,4 @@
-from datetime import date, timedelta
+from datetime import datetime, timedelta
 
 from django.conf import settings
 from django.contrib.auth.models import User
@@ -28,25 +28,28 @@ class TokenGeneratorTest(TestCase):
         self.assertEqual(tk1, tk2)
 
     def test_timeout(self):
-        """
-        The token is valid after n days, but no greater.
-        """
+        """The token is valid after n seconds, but no greater."""
         # Uses a mocked version of PasswordResetTokenGenerator so we can change
-        # the value of 'today'
+        # the value of 'now'.
         class Mocked(PasswordResetTokenGenerator):
-            def __init__(self, today):
-                self._today_val = today
+            def __init__(self, now):
+                self._now_val = now
 
-            def _today(self):
-                return self._today_val
+            def _now(self):
+                return self._now_val
 
         user = User.objects.create_user('tokentestuser', 'test2@example.com', 'testpw')
         p0 = PasswordResetTokenGenerator()
         tk1 = p0.make_token(user)
-        p1 = Mocked(date.today() + timedelta(settings.PASSWORD_RESET_TIMEOUT_DAYS))
+        p1 = Mocked(datetime.now() + timedelta(seconds=settings.PASSWORD_RESET_TIMEOUT))
         self.assertTrue(p1.check_token(user, tk1))
-        p2 = Mocked(date.today() + timedelta(settings.PASSWORD_RESET_TIMEOUT_DAYS + 1))
+        p2 = Mocked(datetime.now() + timedelta(seconds=(settings.PASSWORD_RESET_TIMEOUT + 1)))
         self.assertFalse(p2.check_token(user, tk1))
+        with self.settings(PASSWORD_RESET_TIMEOUT=60 * 60):
+            p3 = Mocked(datetime.now() + timedelta(seconds=settings.PASSWORD_RESET_TIMEOUT))
+            self.assertTrue(p3.check_token(user, tk1))
+            p4 = Mocked(datetime.now() + timedelta(seconds=(settings.PASSWORD_RESET_TIMEOUT + 1)))
+            self.assertFalse(p4.check_token(user, tk1))
 
     def test_check_token_with_nonexistent_token_and_user(self):
         user = User.objects.create_user('tokentestuser', 'test2@example.com', 'testpw')