Browse Source

Fixed #20593 -- Allow blank passwords in check_password() and set_password()

Erik Romijn 11 years ago
parent
commit
2c4fe761a0

+ 6 - 6
django/contrib/auth/hashers.py

@@ -47,7 +47,7 @@ def check_password(password, encoded, setter=None, preferred='default'):
     If setter is specified, it'll be called when you need to
     regenerate the password.
     """
-    if not password or not is_password_usable(encoded):
+    if not is_password_usable(encoded):
         return False
 
     preferred = get_hasher(preferred)
@@ -65,10 +65,10 @@ def make_password(password, salt=None, hasher='default'):
     Turn a plain-text password into a hash for database storage
 
     Same as encode() but generates a new random salt.  If
-    password is None or blank then UNUSABLE_PASSWORD will be
+    password is None then UNUSABLE_PASSWORD will be
     returned which disallows logins.
     """
-    if not password:
+    if password is None:
         return UNUSABLE_PASSWORD
 
     hasher = get_hasher(hasher)
@@ -222,7 +222,7 @@ class PBKDF2PasswordHasher(BasePasswordHasher):
     digest = hashlib.sha256
 
     def encode(self, password, salt, iterations=None):
-        assert password
+        assert password is not None
         assert salt and '$' not in salt
         if not iterations:
             iterations = self.iterations
@@ -350,7 +350,7 @@ class SHA1PasswordHasher(BasePasswordHasher):
     algorithm = "sha1"
 
     def encode(self, password, salt):
-        assert password
+        assert password is not None
         assert salt and '$' not in salt
         hash = hashlib.sha1(force_bytes(salt + password)).hexdigest()
         return "%s$%s$%s" % (self.algorithm, salt, hash)
@@ -378,7 +378,7 @@ class MD5PasswordHasher(BasePasswordHasher):
     algorithm = "md5"
 
     def encode(self, password, salt):
-        assert password
+        assert password is not None
         assert salt and '$' not in salt
         hash = hashlib.md5(force_bytes(salt + password)).hexdigest()
         return "%s$%s$%s" % (self.algorithm, salt, hash)

+ 53 - 0
django/contrib/auth/tests/test_hashers.py

@@ -32,6 +32,12 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(is_password_usable(encoded))
         self.assertTrue(check_password('lètmein', encoded))
         self.assertFalse(check_password('lètmeinz', encoded))
+        # Blank passwords
+        blank_encoded = make_password('')
+        self.assertTrue(blank_encoded.startswith('pbkdf2_sha256$'))
+        self.assertTrue(is_password_usable(blank_encoded))
+        self.assertTrue(check_password('', blank_encoded))
+        self.assertFalse(check_password(' ', blank_encoded))
 
     def test_pkbdf2(self):
         encoded = make_password('lètmein', 'seasalt', 'pbkdf2_sha256')
@@ -41,6 +47,12 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(check_password('lètmein', encoded))
         self.assertFalse(check_password('lètmeinz', encoded))
         self.assertEqual(identify_hasher(encoded).algorithm, "pbkdf2_sha256")
+        # Blank passwords
+        blank_encoded = make_password('', 'seasalt', 'pbkdf2_sha256')
+        self.assertTrue(blank_encoded.startswith('pbkdf2_sha256$'))
+        self.assertTrue(is_password_usable(blank_encoded))
+        self.assertTrue(check_password('', blank_encoded))
+        self.assertFalse(check_password(' ', blank_encoded))
 
     def test_sha1(self):
         encoded = make_password('lètmein', 'seasalt', 'sha1')
@@ -50,6 +62,12 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(check_password('lètmein', encoded))
         self.assertFalse(check_password('lètmeinz', encoded))
         self.assertEqual(identify_hasher(encoded).algorithm, "sha1")
+        # Blank passwords
+        blank_encoded = make_password('', 'seasalt', 'sha1')
+        self.assertTrue(blank_encoded.startswith('sha1$'))
+        self.assertTrue(is_password_usable(blank_encoded))
+        self.assertTrue(check_password('', blank_encoded))
+        self.assertFalse(check_password(' ', blank_encoded))
 
     def test_md5(self):
         encoded = make_password('lètmein', 'seasalt', 'md5')
@@ -59,6 +77,12 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(check_password('lètmein', encoded))
         self.assertFalse(check_password('lètmeinz', encoded))
         self.assertEqual(identify_hasher(encoded).algorithm, "md5")
+        # Blank passwords
+        blank_encoded = make_password('', 'seasalt', 'md5')
+        self.assertTrue(blank_encoded.startswith('md5$'))
+        self.assertTrue(is_password_usable(blank_encoded))
+        self.assertTrue(check_password('', blank_encoded))
+        self.assertFalse(check_password(' ', blank_encoded))
 
     def test_unsalted_md5(self):
         encoded = make_password('lètmein', '', 'unsalted_md5')
@@ -72,6 +96,11 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(is_password_usable(alt_encoded))
         self.assertTrue(check_password('lètmein', alt_encoded))
         self.assertFalse(check_password('lètmeinz', alt_encoded))
+        # Blank passwords
+        blank_encoded = make_password('', '', 'unsalted_md5')
+        self.assertTrue(is_password_usable(blank_encoded))
+        self.assertTrue(check_password('', blank_encoded))
+        self.assertFalse(check_password(' ', blank_encoded))
 
     def test_unsalted_sha1(self):
         encoded = make_password('lètmein', '', 'unsalted_sha1')
@@ -83,6 +112,12 @@ class TestUtilsHashPass(unittest.TestCase):
         # Raw SHA1 isn't acceptable
         alt_encoded = encoded[6:]
         self.assertFalse(check_password('lètmein', alt_encoded))
+        # Blank passwords
+        blank_encoded = make_password('', '', 'unsalted_sha1')
+        self.assertTrue(blank_encoded.startswith('sha1$'))
+        self.assertTrue(is_password_usable(blank_encoded))
+        self.assertTrue(check_password('', blank_encoded))
+        self.assertFalse(check_password(' ', blank_encoded))
 
     @skipUnless(crypt, "no crypt module to generate password.")
     def test_crypt(self):
@@ -92,6 +127,12 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(check_password('lètmei', encoded))
         self.assertFalse(check_password('lètmeiz', encoded))
         self.assertEqual(identify_hasher(encoded).algorithm, "crypt")
+        # Blank passwords
+        blank_encoded = make_password('', 'ab', 'crypt')
+        self.assertTrue(blank_encoded.startswith('crypt$'))
+        self.assertTrue(is_password_usable(blank_encoded))
+        self.assertTrue(check_password('', blank_encoded))
+        self.assertFalse(check_password(' ', blank_encoded))
 
     @skipUnless(bcrypt, "bcrypt not installed")
     def test_bcrypt_sha256(self):
@@ -108,6 +149,12 @@ class TestUtilsHashPass(unittest.TestCase):
         encoded = make_password(password, hasher='bcrypt_sha256')
         self.assertTrue(check_password(password, encoded))
         self.assertFalse(check_password(password[:72], encoded))
+        # Blank passwords
+        blank_encoded = make_password('', hasher='bcrypt_sha256')
+        self.assertTrue(blank_encoded.startswith('bcrypt_sha256$'))
+        self.assertTrue(is_password_usable(blank_encoded))
+        self.assertTrue(check_password('', blank_encoded))
+        self.assertFalse(check_password(' ', blank_encoded))
 
     @skipUnless(bcrypt, "bcrypt not installed")
     def test_bcrypt(self):
@@ -117,6 +164,12 @@ class TestUtilsHashPass(unittest.TestCase):
         self.assertTrue(check_password('lètmein', encoded))
         self.assertFalse(check_password('lètmeinz', encoded))
         self.assertEqual(identify_hasher(encoded).algorithm, "bcrypt")
+        # Blank passwords
+        blank_encoded = make_password('', hasher='bcrypt')
+        self.assertTrue(blank_encoded.startswith('bcrypt$'))
+        self.assertTrue(is_password_usable(blank_encoded))
+        self.assertTrue(check_password('', blank_encoded))
+        self.assertFalse(check_password(' ', blank_encoded))
 
     def test_unusable(self):
         encoded = make_password(None)

+ 16 - 0
docs/ref/contrib/auth.txt

@@ -132,12 +132,28 @@ Methods
         password hashing. Doesn't save the
         :class:`~django.contrib.auth.models.User` object.
 
+        When the ``raw_password`` is ``None``, the password will be set to an
+        unusable password, as if
+        :meth:`~django.contrib.auth.models.User.set_unusable_password()`
+        were used.
+
+        .. versionchanged:: 1.6
+
+            In Django 1.4 and 1.5, a blank string was unintentionally stored
+            as an unsable password.
+
     .. method:: check_password(raw_password)
 
         Returns ``True`` if the given raw string is the correct password for
         the user. (This takes care of the password hashing in making the
         comparison.)
 
+        .. versionchanged:: 1.6
+
+            In Django 1.4 and 1.5, a blank string was unintentionally
+            considered to be an unusable password, resulting in this method
+            returning ``False`` for such a password.
+
     .. method:: set_unusable_password()
 
         Marks the user as having no password set.  This isn't the same as

+ 9 - 0
docs/releases/1.6.txt

@@ -701,6 +701,15 @@ Miscellaneous
 * :class:`~django.views.generic.base.RedirectView` now has a `pattern_name`
   attribute which allows it to choose the target by reversing the URL.
 
+* In Django 1.4 and 1.5, a blank string was unintentionally not considered to
+  be a valid password. This meant
+  :meth:`~django.contrib.auth.models.User.set_password()` would save a blank
+  password as an unusable password like
+  :meth:`~django.contrib.auth.models.User.set_unusable_password()` does, and
+  thus :meth:`~django.contrib.auth.models.User.check_password()` always
+  returned ``False`` for blank passwords. This has been corrected in this
+  release: blank passwords are now valid.
+
 Features deprecated in 1.6
 ==========================
 

+ 16 - 0
docs/topics/auth/customizing.txt

@@ -583,12 +583,28 @@ The following methods are available on any subclass of
         password hashing. Doesn't save the
         :class:`~django.contrib.auth.models.AbstractBaseUser` object.
 
+        When the raw_password is ``None``, the password will be set to an
+        unusable password, as if
+        :meth:`~django.contrib.auth.models.AbstractBaseUser.set_unusable_password()`
+        were used.
+
+        .. versionchanged:: 1.6
+
+            In Django 1.4 and 1.5, a blank string was unintentionally stored
+            as an unsable password as well.
+
     .. method:: models.AbstractBaseUser.check_password(raw_password)
 
         Returns ``True`` if the given raw string is the correct password for
         the user. (This takes care of the password hashing in making the
         comparison.)
 
+        .. versionchanged:: 1.6
+
+            In Django 1.4 and 1.5, a blank string was unintentionally
+            considered to be an unusable password, resulting in this method
+            returning ``False`` for such a password.
+
     .. method:: models.AbstractBaseUser.set_unusable_password()
 
         Marks the user as having no password set.  This isn't the same as

+ 6 - 0
docs/topics/auth/passwords.txt

@@ -206,6 +206,12 @@ from the ``User`` model.
     database to check against, and returns ``True`` if they match, ``False``
     otherwise.
 
+    .. versionchanged:: 1.6
+
+        In Django 1.4 and 1.5, a blank string was unintentionally considered
+        to be an unusable password, resulting in this method returning
+        ``False`` for such a password.
+
 .. function:: make_password(password[, salt, hashers])
 
     Creates a hashed password in the format used by this application. It takes