From e663f695fb282053b31f69ee2dcbc67ddacb9104 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Wed, 11 Mar 2020 09:47:43 +0100 Subject: [PATCH] Fixed #31359 -- Deprecated get_random_string() calls without an explicit length. --- django/contrib/auth/hashers.py | 3 ++- django/db/backends/oracle/creation.py | 2 +- django/utils/crypto.py | 28 ++++++++++++++++++++++----- docs/internals/deprecation.txt | 3 +++ docs/releases/3.1.txt | 3 +++ tests/utils_tests/test_crypto.py | 17 ++++++++++++++-- 6 files changed, 47 insertions(+), 9 deletions(-) diff --git a/django/contrib/auth/hashers.py b/django/contrib/auth/hashers.py index dac1ceacf6..c8cf0d3bf8 100644 --- a/django/contrib/auth/hashers.py +++ b/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.""" diff --git a/django/db/backends/oracle/creation.py b/django/db/backends/oracle/creation.py index ec2667ef33..3ca3754e15 100644 --- a/django/db/backends/oracle/creation.py +++ b/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): diff --git a/django/utils/crypto.py b/django/utils/crypto.py index edeb336f34..8d25d96c0d 100644 --- a/django/utils/crypto.py +++ b/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)) diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index c32666fdb7..580301436a 100644 --- a/docs/internals/deprecation.txt +++ b/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 ` for more details on these changes. diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index 8664cf5d10..18b40b3bcc 100644 --- a/docs/releases/3.1.txt +++ b/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 diff --git a/tests/utils_tests/test_crypto.py b/tests/utils_tests/test_crypto.py index 9dbfd9fe57..4469d6e985 100644 --- a/tests/utils_tests/test_crypto.py +++ b/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()