From dca33ac15de81817c388234323ab358d97055689 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Tue, 18 Nov 2014 21:45:12 +0100 Subject: [PATCH] Simplified caching of password hashers. load_hashers cached its result regardless of its password_hashers argument which required fragile cache invalidation. Remove that argument in favor of @override_settings and triggering cache invalidation with a signal. --- django/contrib/auth/hashers.py | 53 +++++++++++------------ django/contrib/auth/tests/test_hashers.py | 13 +++--- 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/django/contrib/auth/hashers.py b/django/contrib/auth/hashers.py index 184e60b8d2..04f15b0b4f 100644 --- a/django/contrib/auth/hashers.py +++ b/django/contrib/auth/hashers.py @@ -13,22 +13,13 @@ from django.utils.encoding import force_bytes, force_str, force_text from django.core.exceptions import ImproperlyConfigured from django.utils.crypto import ( pbkdf2, constant_time_compare, get_random_string) +from django.utils import lru_cache from django.utils.module_loading import import_string from django.utils.translation import ugettext_noop as _ UNUSABLE_PASSWORD_PREFIX = '!' # This will never be a valid encoded hash UNUSABLE_PASSWORD_SUFFIX_LENGTH = 40 # number of random chars to add after UNUSABLE_PASSWORD_PREFIX -HASHERS = None # lazily loaded from PASSWORD_HASHERS -PREFERRED_HASHER = None # defaults to first item in PASSWORD_HASHERS - - -@receiver(setting_changed) -def reset_hashers(**kwargs): - if kwargs['setting'] == 'PASSWORD_HASHERS': - global HASHERS, PREFERRED_HASHER - HASHERS = None - PREFERRED_HASHER = None def is_password_usable(encoded): @@ -85,20 +76,29 @@ def make_password(password, salt=None, hasher='default'): return hasher.encode(password, salt) -def load_hashers(password_hashers=None): - global HASHERS - global PREFERRED_HASHER +@lru_cache.lru_cache() +def get_hashers(): hashers = [] - if not password_hashers: - password_hashers = settings.PASSWORD_HASHERS - for backend in password_hashers: - hasher = import_string(backend)() + for hasher_path in settings.PASSWORD_HASHERS: + hasher_cls = import_string(hasher_path) + hasher = hasher_cls() if not getattr(hasher, 'algorithm'): raise ImproperlyConfigured("hasher doesn't specify an " - "algorithm name: %s" % backend) + "algorithm name: %s" % hasher_path) hashers.append(hasher) - HASHERS = dict((hasher.algorithm, hasher) for hasher in hashers) - PREFERRED_HASHER = hashers[0] + return hashers + + +@lru_cache.lru_cache() +def get_hashers_by_algorithm(): + return {hasher.algorithm: hasher for hasher in get_hashers()} + + +@receiver(setting_changed) +def reset_hashers(**kwargs): + if kwargs['setting'] == 'PASSWORD_HASHERS': + get_hashers.cache_clear() + get_hashers_by_algorithm.cache_clear() def get_hasher(algorithm='default'): @@ -113,17 +113,16 @@ def get_hasher(algorithm='default'): return algorithm elif algorithm == 'default': - if PREFERRED_HASHER is None: - load_hashers() - return PREFERRED_HASHER + return get_hashers()[0] + else: - if HASHERS is None: - load_hashers() - if algorithm not in HASHERS: + hashers = get_hashers_by_algorithm() + try: + return hashers[algorithm] + except KeyError: raise ValueError("Unknown password hashing algorithm '%s'. " "Did you specify it in the PASSWORD_HASHERS " "setting?" % algorithm) - return HASHERS[algorithm] def identify_hasher(encoded): diff --git a/django/contrib/auth/tests/test_hashers.py b/django/contrib/auth/tests/test_hashers.py index f7ee77aedb..10ab657ae5 100644 --- a/django/contrib/auth/tests/test_hashers.py +++ b/django/contrib/auth/tests/test_hashers.py @@ -3,11 +3,12 @@ from __future__ import unicode_literals from unittest import skipUnless -from django.conf.global_settings import PASSWORD_HASHERS as default_hashers +from django.conf.global_settings import PASSWORD_HASHERS from django.contrib.auth.hashers import (is_password_usable, BasePasswordHasher, - check_password, make_password, PBKDF2PasswordHasher, load_hashers, PBKDF2SHA1PasswordHasher, + check_password, make_password, PBKDF2PasswordHasher, PBKDF2SHA1PasswordHasher, get_hasher, identify_hasher, UNUSABLE_PASSWORD_PREFIX, UNUSABLE_PASSWORD_SUFFIX_LENGTH) from django.test import SimpleTestCase +from django.test.utils import override_settings from django.utils import six @@ -26,11 +27,9 @@ class PBKDF2SingleIterationHasher(PBKDF2PasswordHasher): iterations = 1 +@override_settings(PASSWORD_HASHERS=PASSWORD_HASHERS) class TestUtilsHashPass(SimpleTestCase): - def setUp(self): - load_hashers(password_hashers=default_hashers) - def test_simple(self): encoded = make_password('lètmein') self.assertTrue(encoded.startswith('pbkdf2_sha256$')) @@ -253,8 +252,8 @@ class TestUtilsHashPass(SimpleTestCase): self.assertFalse(state['upgraded']) def test_pbkdf2_upgrade(self): - self.assertEqual('pbkdf2_sha256', get_hasher('default').algorithm) hasher = get_hasher('default') + self.assertEqual('pbkdf2_sha256', hasher.algorithm) self.assertNotEqual(hasher.iterations, 1) old_iterations = hasher.iterations @@ -284,8 +283,8 @@ class TestUtilsHashPass(SimpleTestCase): hasher.iterations = old_iterations def test_pbkdf2_upgrade_new_hasher(self): - self.assertEqual('pbkdf2_sha256', get_hasher('default').algorithm) hasher = get_hasher('default') + self.assertEqual('pbkdf2_sha256', hasher.algorithm) self.assertNotEqual(hasher.iterations, 1) state = {'upgraded': False}