Browse Source

Fixed #31359 -- Deprecated get_random_string() calls without an explicit length.

Claude Paroz 5 years ago
parent
commit
e663f695fb

+ 2 - 1
django/contrib/auth/hashers.py

@@ -185,7 +185,8 @@ class BasePasswordHasher:
 
     def salt(self):
         """Generate a cryptographically secure nonce salt in ASCII."""
-        return get_random_string()
+        # 12 returns a 71-bit value, log_2((26+26+10)^12) =~ 71 bits
+        return get_random_string(12)
 
     def verify(self, password, encoded):
         """Check if the given password is correct."""

+ 1 - 1
django/db/backends/oracle/creation.py

@@ -341,7 +341,7 @@ class DatabaseCreation(BaseDatabaseCreation):
         password = self._test_settings_get('PASSWORD')
         if password is None and self._test_user_create():
             # Oracle passwords are limited to 30 chars and can't contain symbols.
-            password = get_random_string(length=30)
+            password = get_random_string(30)
         return password
 
     def _test_database_tblspace(self):

+ 23 - 5
django/utils/crypto.py

@@ -4,8 +4,10 @@ Django's standard crypto functions and utilities.
 import hashlib
 import hmac
 import secrets
+import warnings
 
 from django.conf import settings
+from django.utils.deprecation import RemovedInDjango40Warning
 from django.utils.encoding import force_bytes
 
 
@@ -44,15 +46,31 @@ def salted_hmac(key_salt, value, secret=None, *, algorithm='sha1'):
     return hmac.new(key, msg=force_bytes(value), digestmod=hasher)
 
 
-def get_random_string(length=12,
-                      allowed_chars='abcdefghijklmnopqrstuvwxyz'
-                                    'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'):
+NOT_PROVIDED = object()  # RemovedInDjango40Warning.
+
+
+# RemovedInDjango40Warning: when the deprecation ends, replace with:
+#   def get_random_string(self, length, allowed_chars='...'):
+def get_random_string(length=NOT_PROVIDED, allowed_chars=(
+    'abcdefghijklmnopqrstuvwxyz'
+    'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'
+)):
     """
     Return a securely generated random string.
 
-    The default length of 12 with the a-z, A-Z, 0-9 character set returns
-    a 71-bit value. log_2((26+26+10)^12) =~ 71 bits
+    The bit length of the returned value can be calculated with the formula:
+        log_2(len(allowed_chars)^length)
+
+    For example, with default `allowed_chars` (26+26+10), this gives:
+      * length: 12, bit length =~ 71 bits
+      * length: 22, bit length =~ 131 bits
     """
+    if length is NOT_PROVIDED:
+        warnings.warn(
+            'Not providing a length argument is deprecated.',
+            RemovedInDjango40Warning,
+        )
+        length = 12
     return ''.join(secrets.choice(allowed_chars) for i in range(length))
 
 

+ 3 - 0
docs/internals/deprecation.txt

@@ -61,6 +61,9 @@ details on these changes.
 * The ``providing_args`` argument for ``django.dispatch.Signal`` will be
   removed.
 
+* The ``length`` argument for ``django.utils.crypto.get_random_string()`` will
+  be required.
+
 See the :ref:`Django 3.1 release notes <deprecated-features-3.1>` for more
 details on these changes.
 

+ 3 - 0
docs/releases/3.1.txt

@@ -554,6 +554,9 @@ Miscellaneous
   argument as documentation, you can move the text to a code comment or
   docstring.
 
+* Calling ``django.utils.crypto.get_random_string()`` without a ``length``
+  argument is deprecated.
+
 .. _removed-features-3.1:
 
 Features removed in 3.1

+ 15 - 2
tests/utils_tests/test_crypto.py

@@ -1,10 +1,12 @@
 import hashlib
 import unittest
 
-from django.test import SimpleTestCase
+from django.test import SimpleTestCase, ignore_warnings
 from django.utils.crypto import (
-    InvalidAlgorithm, constant_time_compare, pbkdf2, salted_hmac,
+    InvalidAlgorithm, constant_time_compare, get_random_string, pbkdf2,
+    salted_hmac,
 )
+from django.utils.deprecation import RemovedInDjango40Warning
 
 
 class TestUtilsCryptoMisc(SimpleTestCase):
@@ -183,3 +185,14 @@ class TestUtilsCryptoPBKDF2(unittest.TestCase):
     def test_default_hmac_alg(self):
         kwargs = {'password': b'password', 'salt': b'salt', 'iterations': 1, 'dklen': 20}
         self.assertEqual(pbkdf2(**kwargs), hashlib.pbkdf2_hmac(hash_name=hashlib.sha256().name, **kwargs))
+
+
+class DeprecationTests(SimpleTestCase):
+    @ignore_warnings(category=RemovedInDjango40Warning)
+    def test_get_random_string(self):
+        self.assertEqual(len(get_random_string()), 12)
+
+    def test_get_random_string_warning(self):
+        msg = 'Not providing a length argument is deprecated.'
+        with self.assertRaisesMessage(RemovedInDjango40Warning, msg):
+            get_random_string()