Browse Source

Fixed #28718 -- Allowed user to request a password reset if their password doesn't use an enabled hasher.

Regression in aeb1389442d0f9669edf6660b747fd10693b63a7.
Reverted changes to is_password_usable() from
703c266682be39f7153498ad0d8031231f12ee79 and documentation changes from
92f48680dbd2e02f2b33f6ad0e35b7d337889fb2.
Tim Graham 7 years ago
parent
commit
a4f0e9aec7

+ 1 - 3
django/contrib/auth/base_user.py

@@ -116,9 +116,7 @@ class AbstractBaseUser(models.Model):
 
     def has_usable_password(self):
         """
-        Return False if set_unusable_password() has been called for this user,
-        or if the password is None, or if the password uses a hasher that's not
-        in the PASSWORD_HASHERS setting.
+        Return False if set_unusable_password() has been called for this user.
         """
         return is_password_usable(self.password)
 

+ 10 - 8
django/contrib/auth/hashers.py

@@ -21,13 +21,11 @@ UNUSABLE_PASSWORD_SUFFIX_LENGTH = 40  # number of random chars to add after UNUS
 
 
 def is_password_usable(encoded):
-    if encoded is None or encoded.startswith(UNUSABLE_PASSWORD_PREFIX):
-        return False
-    try:
-        identify_hasher(encoded)
-    except ValueError:
-        return False
-    return True
+    """
+    Return True if this password wasn't generated by
+    User.set_unusable_password(), i.e. make_password(None).
+    """
+    return encoded is None or not encoded.startswith(UNUSABLE_PASSWORD_PREFIX)
 
 
 def check_password(password, encoded, setter=None, preferred='default'):
@@ -42,7 +40,11 @@ def check_password(password, encoded, setter=None, preferred='default'):
         return False
 
     preferred = get_hasher(preferred)
-    hasher = identify_hasher(encoded)
+    try:
+        hasher = identify_hasher(encoded)
+    except ValueError:
+        # encoded is gibberish or uses a hasher that's no longer installed.
+        return False
 
     hasher_changed = hasher.algorithm != preferred.algorithm
     must_update = hasher_changed or preferred.must_update(encoded)

+ 9 - 3
docs/ref/contrib/auth.txt

@@ -212,9 +212,15 @@ Methods
 
         Returns ``False`` if
         :meth:`~django.contrib.auth.models.User.set_unusable_password()` has
-        been called for this user, or if the password is ``None``, or if the
-        password uses a hasher that's not in the :setting:`PASSWORD_HASHERS`
-        setting.
+        been called for this user.
+
+        .. versionchanged:: 2.1
+
+            In older versions, this also returns ``False`` if the password is
+            ``None`` or an empty string, or if the password uses a hasher
+            that's not in the :setting:`PASSWORD_HASHERS` setting. That
+            behavior is considered a bug as it prevents users with such
+            passwords from requesting a password reset.
 
     .. method:: get_group_permissions(obj=None)
 

+ 8 - 0
docs/releases/2.1.txt

@@ -358,6 +358,14 @@ Miscellaneous
   changed from 0 to an empty string, which mainly may require some adjustments
   in tests that compare HTML.
 
+* :meth:`.User.has_usable_password` and the
+  :func:`~django.contrib.auth.hashers.is_password_usable` function no longer
+  return ``False`` if the password is ``None`` or an empty string, or if the
+  password uses a hasher that's not in the :setting:`PASSWORD_HASHERS` setting.
+  This undocumented behavior was a regression in Django 1.6 and prevented users
+  with such passwords from requesting a password reset. Audit your code to
+  confirm that your usage of these APIs don't rely on the old behavior.
+
 .. _deprecated-features-2.1:
 
 Features deprecated in 2.1

+ 10 - 2
docs/topics/auth/passwords.txt

@@ -409,8 +409,16 @@ from the ``User`` model.
 
 .. function:: is_password_usable(encoded_password)
 
-   Checks if the given string is a hashed password that has a chance
-   of being verified against :func:`check_password`.
+    Returns ``False`` if the password is a result of
+    :meth:`.User.set_unusable_password`.
+
+    .. versionchanged:: 2.1
+
+        In older versions, this also returns ``False`` if the password is
+        ``None`` or an empty string, or if the password uses a hasher that's
+        not in the :setting:`PASSWORD_HASHERS` setting. That behavior is
+        considered a bug as it prevents users with such passwords from
+        requesting a password reset.
 
 .. _password-validation:
 

+ 5 - 3
tests/auth_tests/test_hashers.py

@@ -276,9 +276,11 @@ class TestUtilsHashPass(SimpleTestCase):
         with self.assertRaisesMessage(ValueError, msg % 'lolcat'):
             identify_hasher('lolcat$salt$hash')
 
-    def test_bad_encoded(self):
-        self.assertFalse(is_password_usable('lètmein_badencoded'))
-        self.assertFalse(is_password_usable(''))
+    def test_is_password_usable(self):
+        passwords = ('lètmein_badencoded', '', None)
+        for password in passwords:
+            with self.subTest(password=password):
+                self.assertIs(is_password_usable(password), True)
 
     def test_low_level_pbkdf2(self):
         hasher = PBKDF2PasswordHasher()

+ 7 - 0
tests/auth_tests/test_models.py

@@ -158,6 +158,13 @@ class UserManagerTestCase(TestCase):
 
 class AbstractBaseUserTests(TestCase):
 
+    def test_has_usable_password(self):
+        """
+        Passwords are usable even if they don't correspond to a hasher in
+        settings.PASSWORD_HASHERS.
+        """
+        self.assertIs(User(password='some-gibbberish').has_usable_password(), True)
+
     def test_normalize_username(self):
         self.assertEqual(IntegerUsernameUser().normalize_username(123), 123)