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

This commit is contained in:
Hasan Ramezani 2019-08-23 17:14:07 +02:00 committed by Mariusz Felisiak
parent 0719edcd5f
commit 226ebb1729
8 changed files with 178 additions and 27 deletions

View File

@ -9,14 +9,23 @@ for a list of all possible variables.
import importlib import importlib
import os import os
import time import time
import traceback
import warnings
from pathlib import Path from pathlib import Path
import django
from django.conf import global_settings from django.conf import global_settings
from django.core.exceptions import ImproperlyConfigured from django.core.exceptions import ImproperlyConfigured
from django.utils.deprecation import RemovedInDjango40Warning
from django.utils.functional import LazyObject, empty from django.utils.functional import LazyObject, empty
ENVIRONMENT_VARIABLE = "DJANGO_SETTINGS_MODULE" 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): class SettingsReference(str):
""" """
@ -105,6 +114,20 @@ class LazySettings(LazyObject):
"""Return True if the settings have already been configured.""" """Return True if the settings have already been configured."""
return self._wrapped is not empty 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: class Settings:
def __init__(self, settings_module): def __init__(self, settings_module):
@ -137,6 +160,15 @@ class Settings:
if not self.SECRET_KEY: if not self.SECRET_KEY:
raise ImproperlyConfigured("The SECRET_KEY setting must not be empty.") 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: if hasattr(time, 'tzset') and self.TIME_ZONE:
# When we can, attempt to validate the timezone. If we can't find # When we can, attempt to validate the timezone. If we can't find
# this file, no check happens and it's harmless. # this file, no check happens and it's harmless.
@ -180,6 +212,9 @@ class UserSettingsHolder:
def __setattr__(self, name, value): def __setattr__(self, name, value):
self._deleted.discard(name) 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) super().__setattr__(name, value)
def __delattr__(self, name): def __delattr__(self, name):

View File

@ -506,6 +506,10 @@ LOGOUT_REDIRECT_URL = None
# The number of days a password reset link is valid for # The number of days a password reset link is valid for
PASSWORD_RESET_TIMEOUT_DAYS = 3 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 # the first hasher in this list is the preferred algorithm. any
# password using different algorithms will be converted automatically # password using different algorithms will be converted automatically
# upon login # upon login

View File

@ -1,4 +1,4 @@
from datetime import date from datetime import datetime
from django.conf import settings from django.conf import settings
from django.utils.crypto import constant_time_compare, salted_hmac 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 Return a token that can be used once to do a password reset
for the given user. 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): 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): if not constant_time_compare(self._make_token_with_timestamp(user, ts), token):
return False return False
# Check the timestamp is within limit. Timestamps are rounded to # Check the timestamp is within limit.
# midnight (server time) providing a resolution of only 1 day. If a if (self._num_seconds(self._now()) - ts) > settings.PASSWORD_RESET_TIMEOUT:
# 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:
return False return False
return True return True
def _make_token_with_timestamp(self, user, timestamp): def _make_token_with_timestamp(self, user, timestamp):
# timestamp is number of days since 2001-1-1. Converted to # timestamp is number of seconds since 2001-1-1. Converted to base 36,
# base 36, this gives us a 3 digit string until about 2121 # this gives us a 6 digit string until about 2069.
ts_b36 = int_to_base36(timestamp) ts_b36 = int_to_base36(timestamp)
hash_string = salted_hmac( hash_string = salted_hmac(
self.key_salt, self.key_salt,
@ -71,7 +67,7 @@ class PasswordResetTokenGenerator:
same password is chosen, due to password salting). same password is chosen, due to password salting).
2. The last_login field will usually be updated very shortly after 2. The last_login field will usually be updated very shortly after
a password reset. a password reset.
Failing those things, settings.PASSWORD_RESET_TIMEOUT_DAYS eventually Failing those things, settings.PASSWORD_RESET_TIMEOUT eventually
invalidates the token. invalidates the token.
Running this data through salted_hmac() prevents password cracking 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) 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) return str(user.pk) + user.password + str(login_timestamp) + str(timestamp)
def _num_days(self, dt): def _num_seconds(self, dt):
return (dt - date(2001, 1, 1)).days return int((dt - datetime(2001, 1, 1)).total_seconds())
def _today(self): def _now(self):
# Used for mocking in tests # Used for mocking in tests
return date.today() return datetime.now()
default_token_generator = PasswordResetTokenGenerator() default_token_generator = PasswordResetTokenGenerator()

View File

@ -34,6 +34,8 @@ details on these changes.
* ``django.utils.http.is_safe_url()`` will be removed. * ``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 See the :ref:`Django 3.1 release notes <deprecated-features-3.1>` for more
details on these changes. details on these changes.

View File

@ -2872,6 +2872,19 @@ doesn't have a ``next_page`` attribute.
If ``None``, no redirect will be performed and the logout view will be If ``None``, no redirect will be performed and the logout view will be
rendered. 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 .. setting:: PASSWORD_RESET_TIMEOUT_DAYS
``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`. 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 .. setting:: PASSWORD_HASHERS
``PASSWORD_HASHERS`` ``PASSWORD_HASHERS``

View File

@ -46,6 +46,11 @@ Minor features
* The default iteration count for the PBKDF2 password hasher is increased from * The default iteration count for the PBKDF2 password hasher is increased from
180,000 to 216,000. 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` :mod:`django.contrib.contenttypes`
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@ -222,7 +227,8 @@ Features deprecated in 3.1
Miscellaneous Miscellaneous
------------- -------------
* ... * ``PASSWORD_RESET_TIMEOUT_DAYS`` setting is deprecated in favor of
:setting:`PASSWORD_RESET_TIMEOUT`.
.. _removed-features-3.1: .. _removed-features-3.1:

View File

@ -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']

View File

@ -1,4 +1,4 @@
from datetime import date, timedelta from datetime import datetime, timedelta
from django.conf import settings from django.conf import settings
from django.contrib.auth.models import User from django.contrib.auth.models import User
@ -28,25 +28,28 @@ class TokenGeneratorTest(TestCase):
self.assertEqual(tk1, tk2) self.assertEqual(tk1, tk2)
def test_timeout(self): def test_timeout(self):
""" """The token is valid after n seconds, but no greater."""
The token is valid after n days, but no greater.
"""
# Uses a mocked version of PasswordResetTokenGenerator so we can change # Uses a mocked version of PasswordResetTokenGenerator so we can change
# the value of 'today' # the value of 'now'.
class Mocked(PasswordResetTokenGenerator): class Mocked(PasswordResetTokenGenerator):
def __init__(self, today): def __init__(self, now):
self._today_val = today self._now_val = now
def _today(self): def _now(self):
return self._today_val return self._now_val
user = User.objects.create_user('tokentestuser', 'test2@example.com', 'testpw') user = User.objects.create_user('tokentestuser', 'test2@example.com', 'testpw')
p0 = PasswordResetTokenGenerator() p0 = PasswordResetTokenGenerator()
tk1 = p0.make_token(user) 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)) 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)) 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): def test_check_token_with_nonexistent_token_and_user(self):
user = User.objects.create_user('tokentestuser', 'test2@example.com', 'testpw') user = User.objects.create_user('tokentestuser', 'test2@example.com', 'testpw')