Fixed #33199 -- Deprecated passing positional arguments to Signer/TimestampSigner.

Thanks Jacob Walls for the implementation idea.

AUTHORS

@@ -11,6 +11,7 @@ answer newbie questions, and generally made Django that much better:
     Abeer Upadhyay <ab.esquarer@gmail.com>
     Abhijeet Viswa <abhijeetviswa@gmail.com>
     Abhinav Patil <https://github.com/ubadub/>
+    Abhinav Yadav <abhinav.sny.2002+django@gmail.com>
     Abhishek Gautam <abhishekg1128@yahoo.com>
     Abhyudai <https://github.com/abhiabhi94>
     Adam Allred <adam.w.allred@gmail.com>

django/core/signing.py

@@ -37,10 +37,12 @@ import base64
 import datetime
 import json
 import time
+import warnings
 import zlib
 
 from django.conf import settings
 from django.utils.crypto import constant_time_compare, salted_hmac
+from django.utils.deprecation import RemovedInDjango51Warning
 from django.utils.encoding import force_bytes
 from django.utils.module_loading import import_string
 from django.utils.regex_helper import _lazy_re_compile
@@ -147,7 +149,7 @@ def dumps(
 
     The serializer is expected to return a bytestring.
     """
-    return TimestampSigner(key, salt=salt).sign_object(
+    return TimestampSigner(key=key, salt=salt).sign_object(
         obj, serializer=serializer, compress=compress
     )
 
@@ -165,7 +167,9 @@ def loads(
 
     The serializer is expected to accept a bytestring.
     """
-    return TimestampSigner(key, salt=salt, fallback_keys=fallback_keys).unsign_object(
+    return TimestampSigner(
+        key=key, salt=salt, fallback_keys=fallback_keys
+    ).unsign_object(
         s,
         serializer=serializer,
         max_age=max_age,
@@ -173,8 +177,13 @@ def loads(
 
 
 class Signer:
+    # RemovedInDjango51Warning: When the deprecation ends, replace with:
+    # def __init__(
+    #   self, *, key=None, sep=":", salt=None, algorithm=None, fallback_keys=None
+    # ):
     def __init__(
         self,
+        *args,
         key=None,
         sep=":",
         salt=None,
@@ -188,16 +197,29 @@ class Signer:
             else settings.SECRET_KEY_FALLBACKS
         )
         self.sep = sep
-        if _SEP_UNSAFE.match(self.sep):
-            raise ValueError(
-                "Unsafe Signer separator: %r (cannot be empty or consist of "
-                "only A-z0-9-_=)" % sep,
-            )
         self.salt = salt or "%s.%s" % (
             self.__class__.__module__,
             self.__class__.__name__,
         )
         self.algorithm = algorithm or "sha256"
+        # RemovedInDjango51Warning.
+        if args:
+            warnings.warn(
+                f"Passing positional arguments to {self.__class__.__name__} is "
+                f"deprecated.",
+                RemovedInDjango51Warning,
+                stacklevel=2,
+            )
+            for arg, attr in zip(
+                args, ["key", "sep", "salt", "algorithm", "fallback_keys"]
+            ):
+                if arg or attr == "sep":
+                    setattr(self, attr, arg)
+        if _SEP_UNSAFE.match(self.sep):
+            raise ValueError(
+                "Unsafe Signer separator: %r (cannot be empty or consist of "
+                "only A-z0-9-_=)" % sep,
+            )
 
     def signature(self, value, key=None):
         key = key or self.key

docs/internals/deprecation.txt

@@ -42,6 +42,9 @@ details on these changes.
 * Support for passing encoded JSON string literals to ``JSONField`` and
   associated lookups and expressions will be removed.
 
+* Support for passing positional arguments to ``Signer`` and
+  ``TimestampSigner`` will be removed.
+
 .. _deprecation-removed-in-5.0:
 
 5.0

docs/releases/4.2.txt

@@ -540,3 +540,6 @@ Miscellaneous
 
 * ``TransactionTestCase.assertQuerysetEqual()`` is deprecated in favor of
   ``assertQuerySetEqual()``.
+
+* Passing positional arguments to ``Signer`` and ``TimestampSigner`` is
+  deprecated in favor of keyword-only arguments.

docs/topics/signing.txt

@@ -96,12 +96,12 @@ By default, the ``Signer`` class uses the :setting:`SECRET_KEY` setting to
 generate signatures. You can use a different secret by passing it to the
 ``Signer`` constructor::
 
-    >>> signer = Signer('my-other-secret')
+    >>> signer = Signer(key='my-other-secret')
     >>> value = signer.sign('My string')
     >>> value
     'My string:EkfQJafvGyiofrdGnuthdxImIJw'
 
-.. class:: Signer(key=None, sep=':', salt=None, algorithm=None, fallback_keys=None)
+.. class:: Signer(*, key=None, sep=':', salt=None, algorithm=None, fallback_keys=None)
 
     Returns a signer which uses ``key`` to generate signatures and ``sep`` to
     separate values. ``sep`` cannot be in the :rfc:`URL safe base64 alphabet
@@ -115,6 +115,10 @@ generate signatures. You can use a different secret by passing it to the
 
         The ``fallback_keys`` argument was added.
 
+    .. deprecated:: 4.2
+
+        Support for passing positional arguments is deprecated.
+
 Using the ``salt`` argument
 ---------------------------
 
@@ -172,7 +176,7 @@ created within a specified period of time::
     >>> signer.unsign(value, max_age=timedelta(seconds=20))
     'hello'
 
-.. class:: TimestampSigner(key=None, sep=':', salt=None, algorithm='sha256')
+.. class:: TimestampSigner(*, key=None, sep=':', salt=None, algorithm='sha256')
 
     .. method:: sign(value)
 
@@ -195,6 +199,10 @@ created within a specified period of time::
         otherwise raises ``SignatureExpired``. The ``max_age`` parameter can
         accept an integer or a :py:class:`datetime.timedelta` object.
 
+    .. deprecated:: 4.2
+
+        Support for passing positional arguments is deprecated.
+
 .. _signing-complex-data:
 
 Protecting complex data structures

tests/signing/tests.py

@@ -2,15 +2,16 @@ import datetime
 
 from django.core import signing
 from django.test import SimpleTestCase, override_settings
-from django.test.utils import freeze_time
+from django.test.utils import freeze_time, ignore_warnings
 from django.utils.crypto import InvalidAlgorithm
+from django.utils.deprecation import RemovedInDjango51Warning
 
 
 class TestSigner(SimpleTestCase):
     def test_signature(self):
         "signature() method should generate a signature"
-        signer = signing.Signer("predictable-secret")
+        signer = signing.Signer(key="predictable-secret")
-        signer2 = signing.Signer("predictable-secret2")
+        signer2 = signing.Signer(key="predictable-secret2")
         for s in (
             b"hello",
             b"3098247:529:087:",
@@ -28,8 +29,7 @@ class TestSigner(SimpleTestCase):
             self.assertNotEqual(signer.signature(s), signer2.signature(s))
 
     def test_signature_with_salt(self):
-        "signature(value, salt=...) should work"
+        signer = signing.Signer(key="predictable-secret", salt="extra-salt")
-        signer = signing.Signer("predictable-secret", salt="extra-salt")
         self.assertEqual(
             signer.signature("hello"),
             signing.base64_hmac(
@@ -40,12 +40,12 @@ class TestSigner(SimpleTestCase):
             ),
         )
         self.assertNotEqual(
-            signing.Signer("predictable-secret", salt="one").signature("hello"),
+            signing.Signer(key="predictable-secret", salt="one").signature("hello"),
-            signing.Signer("predictable-secret", salt="two").signature("hello"),
+            signing.Signer(key="predictable-secret", salt="two").signature("hello"),
         )
 
     def test_custom_algorithm(self):
-        signer = signing.Signer("predictable-secret", algorithm="sha512")
+        signer = signing.Signer(key="predictable-secret", algorithm="sha512")
         self.assertEqual(
             signer.signature("hello"),
             "Usf3uVQOZ9m6uPfVonKR-EBXjPe7bjMbp3_Fq8MfsptgkkM1ojidN0BxYaT5HAEN1"
@@ -53,14 +53,14 @@ class TestSigner(SimpleTestCase):
         )
 
     def test_invalid_algorithm(self):
-        signer = signing.Signer("predictable-secret", algorithm="whatever")
+        signer = signing.Signer(key="predictable-secret", algorithm="whatever")
         msg = "'whatever' is not an algorithm accepted by the hashlib module."
         with self.assertRaisesMessage(InvalidAlgorithm, msg):
             signer.sign("hello")
 
     def test_sign_unsign(self):
         "sign/unsign should be reversible"
-        signer = signing.Signer("predictable-secret")
+        signer = signing.Signer(key="predictable-secret")
         examples = [
             "q;wjmbk;wkmb",
             "3098247529087",
@@ -75,7 +75,7 @@ class TestSigner(SimpleTestCase):
             self.assertEqual(example, signer.unsign(signed))
 
     def test_sign_unsign_non_string(self):
-        signer = signing.Signer("predictable-secret")
+        signer = signing.Signer(key="predictable-secret")
         values = [
             123,
             1.23,
@@ -91,7 +91,7 @@ class TestSigner(SimpleTestCase):
 
     def test_unsign_detects_tampering(self):
         "unsign should raise an exception if the value has been tampered with"
-        signer = signing.Signer("predictable-secret")
+        signer = signing.Signer(key="predictable-secret")
         value = "Another string"
         signed_value = signer.sign(value)
         transforms = (
@@ -106,7 +106,7 @@ class TestSigner(SimpleTestCase):
                 signer.unsign(transform(signed_value))
 
     def test_sign_unsign_object(self):
-        signer = signing.Signer("predictable-secret")
+        signer = signing.Signer(key="predictable-secret")
         tests = [
             ["a", "list"],
             "a string \u2019",
@@ -155,7 +155,7 @@ class TestSigner(SimpleTestCase):
     def test_works_with_non_ascii_keys(self):
         binary_key = b"\xe7"  # Set some binary (non-ASCII key)
 
-        s = signing.Signer(binary_key)
+        s = signing.Signer(key=binary_key)
         self.assertEqual(
             "foo:EE4qGC5MEKyQG5msxYA0sBohAxLC0BJf8uRhemh0BGU",
             s.sign("foo"),
@@ -164,7 +164,7 @@ class TestSigner(SimpleTestCase):
     def test_valid_sep(self):
         separators = ["/", "*sep*", ","]
         for sep in separators:
-            signer = signing.Signer("predictable-secret", sep=sep)
+            signer = signing.Signer(key="predictable-secret", sep=sep)
             self.assertEqual(
                 "foo%sjZQoX_FtSO70jX9HLRGg2A_2s4kdDBxz1QoO_OpEQb0" % sep,
                 signer.sign("foo"),
@@ -181,16 +181,16 @@ class TestSigner(SimpleTestCase):
                 signing.Signer(sep=sep)
 
     def test_verify_with_non_default_key(self):
-        old_signer = signing.Signer("secret")
+        old_signer = signing.Signer(key="secret")
         new_signer = signing.Signer(
-            "newsecret", fallback_keys=["othersecret", "secret"]
+            key="newsecret", fallback_keys=["othersecret", "secret"]
         )
         signed = old_signer.sign("abc")
         self.assertEqual(new_signer.unsign(signed), "abc")
 
     def test_sign_unsign_multiple_keys(self):
         """The default key is a valid verification key."""
-        signer = signing.Signer("secret", fallback_keys=["oldsecret"])
+        signer = signing.Signer(key="secret", fallback_keys=["oldsecret"])
         signed = signer.sign("abc")
         self.assertEqual(signer.unsign(signed), "abc")
 
@@ -199,7 +199,7 @@ class TestSigner(SimpleTestCase):
         SECRET_KEY_FALLBACKS=["oldsecret"],
     )
     def test_sign_unsign_ignore_secret_key_fallbacks(self):
-        old_signer = signing.Signer("oldsecret")
+        old_signer = signing.Signer(key="oldsecret")
         signed = old_signer.sign("abc")
         signer = signing.Signer(fallback_keys=[])
         with self.assertRaises(signing.BadSignature):
@@ -210,7 +210,7 @@ class TestSigner(SimpleTestCase):
         SECRET_KEY_FALLBACKS=["oldsecret"],
     )
     def test_default_keys_verification(self):
-        old_signer = signing.Signer("oldsecret")
+        old_signer = signing.Signer(key="oldsecret")
         signed = old_signer.sign("abc")
         signer = signing.Signer()
         self.assertEqual(signer.unsign(signed), "abc")
@@ -220,9 +220,9 @@ class TestTimestampSigner(SimpleTestCase):
     def test_timestamp_signer(self):
         value = "hello"
         with freeze_time(123456789):
-            signer = signing.TimestampSigner("predictable-key")
+            signer = signing.TimestampSigner(key="predictable-key")
             ts = signer.sign(value)
-            self.assertNotEqual(ts, signing.Signer("predictable-key").sign(value))
+            self.assertNotEqual(ts, signing.Signer(key="predictable-key").sign(value))
             self.assertEqual(signer.unsign(ts), value)
 
         with freeze_time(123456800):
@@ -240,3 +240,23 @@ class TestBase62(SimpleTestCase):
         tests = [-(10**10), 10**10, 1620378259, *range(-100, 100)]
         for i in tests:
             self.assertEqual(i, signing.b62_decode(signing.b62_encode(i)))
+
+
+class SignerPositionalArgumentsDeprecationTests(SimpleTestCase):
+    def test_deprecation(self):
+        msg = "Passing positional arguments to Signer is deprecated."
+        with self.assertRaisesMessage(RemovedInDjango51Warning, msg):
+            signing.Signer("predictable-secret")
+        msg = "Passing positional arguments to TimestampSigner is deprecated."
+        with self.assertRaisesMessage(RemovedInDjango51Warning, msg):
+            signing.TimestampSigner("predictable-secret")
+
+    @ignore_warnings(category=RemovedInDjango51Warning)
+    def test_positional_arguments(self):
+        signer = signing.Signer("secret", "/", "somesalt", "sha1", ["oldsecret"])
+        signed = signer.sign("xyz")
+        self.assertEqual(signed, "xyz/zzdO_8rk-NGnm8jNasXRTF2P5kY")
+        self.assertEqual(signer.unsign(signed), "xyz")
+        old_signer = signing.Signer("oldsecret", "/", "somesalt", "sha1")
+        signed = old_signer.sign("xyz")
+        self.assertEqual(signer.unsign(signed), "xyz")