Repository: jpadilla/pyjwt
Pull request #776 (2022-07-03 22:11:44)

Emit a deprecation warning for unsupported kwargs
This is half of #258. It cannot be completed without a breaking release, but the API should be reconsidered at that time because it doesn't make a ton of sense for validation-specific arguments like `leeway` to be presented as args to `decode()`.

---

`**kwargs` usages cannot be removed without breaking backwards compatibility. Unsupported kwargs cannot even be rejected without breaking compatibility.

However, this does not mean that the library cannot identify and warn when unsupported arguments are used. The warning behavior simply has to be separated from any removal of `**kwargs`.

All legitimate `**kwargs` usages have been replaced with explicit arguments. Any other arguments will be captured under `**kwargs` and trigger the deprecation warnings. In the cases of `decode() -> decode_complete()` passthrough, the passthrough has been removed to avoid duplicate deprecation warnings on a single usage.

This makes a very subtle behavioral change to `**kwargs` *only* for the case of a subclass of PyJWT or PyJWS. Extra arguments used by a specialized subclass won't pass through transparently anymore. In such a case the subclass author has multiple resolutions available, including reimplementation of the `decode()` method to passthrough the additional argument. Although technically backwards-incompatible for a niche subclassing usage, this behavior is very nearly identical and shouldn't pose an issue for the vast majority of pyjwt users.

The deprecation warning does not cover all deprecated usages. In particular, several passthrough arguments for claim validation should probably be made available via `options` and later removed. The arguments in need of attention now have inline comments in the signature definitions, but are otherwise left unmodified, leaving current usages correct and valid.

sirosen: The conflict was just changelog lines for the Unreleased section. I've resolved with a rebase.

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 45f263a..4ea8dea 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,6 +1,6 @@
 repos:
   - repo: https://github.com/psf/black
-    rev: 22.3.0
+    rev: 22.6.0
     hooks:
       - id: black
         args: ["--target-version=py37"]
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 3ed6559..fb88a29 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -11,6 +11,9 @@ Changed
 ~~~~~~~
 - Skip keys with incompatible alg when loading JWKSet by @DaGuich in https://github.com/jpadilla/pyjwt/pull/762
 - Remove support for python3.6
+- PyJWT now emits a warning for unsupported keyword arguments being passed to
+  ``decode`` and ``decode_complete``. Additional keyword arguments are still
+  supported, but will be rejected in a future version.
 
 Fixed
 ~~~~~
diff --git a/jwt/api_jws.py b/jwt/api_jws.py
index f8c8048..75f826d 100644
--- a/jwt/api_jws.py
+++ b/jwt/api_jws.py
@@ -1,5 +1,6 @@
 import binascii
 import json
+import warnings
 from collections.abc import Mapping
 from typing import Any, Dict, List, Optional, Type
 
@@ -16,6 +17,7 @@ from .exceptions import (
     InvalidTokenError,
 )
 from .utils import base64url_decode, base64url_encode
+from .warnings import RemovedInPyjwt3Warning
 
 
 class PyJWS:
@@ -167,6 +169,13 @@ class PyJWS:
         detached_payload: Optional[bytes] = None,
         **kwargs,
     ) -> Dict[str, Any]:
+        if kwargs:
+            warnings.warn(
+                "passing additional kwargs to decode_complete() is deprecated "
+                "and will be removed in pyjwt version 3. "
+                f"Unsupported kwargs: {tuple(kwargs.keys())}",
+                RemovedInPyjwt3Warning,
+            )
         if options is None:
             options = {}
         merged_options = {**self.options, **options}
@@ -202,9 +211,19 @@ class PyJWS:
         key: str = "",
         algorithms: Optional[List[str]] = None,
         options: Optional[Dict[str, Any]] = None,
+        detached_payload: Optional[bytes] = None,
         **kwargs,
     ) -> str:
-        decoded = self.decode_complete(jwt, key, algorithms, options, **kwargs)
+        if kwargs:
+            warnings.warn(
+                "passing additional kwargs to decode() is deprecated "
+                "and will be removed in pyjwt version 3. "
+                f"Unsupported kwargs: {tuple(kwargs.keys())}",
+                RemovedInPyjwt3Warning,
+            )
+        decoded = self.decode_complete(
+            jwt, key, algorithms, options, detached_payload=detached_payload
+        )
         return decoded["payload"]
 
     def get_unverified_header(self, jwt):
diff --git a/jwt/api_jwt.py b/jwt/api_jwt.py
index a011c0f..b08e950 100644
--- a/jwt/api_jwt.py
+++ b/jwt/api_jwt.py
@@ -15,6 +15,7 @@ from .exceptions import (
     InvalidIssuerError,
     MissingRequiredClaimError,
 )
+from .warnings import RemovedInPyjwt3Warning
 
 
 class PyJWT:
@@ -69,15 +70,32 @@ class PyJWT:
         key: str = "",
         algorithms: Optional[List[str]] = None,
         options: Optional[Dict[str, Any]] = None,
+        # deprecated arg, remove in pyjwt3
+        verify: Optional[bool] = None,
+        # could be used as passthrough to api_jws, consider removal in pyjwt3
+        detached_payload: Optional[bytes] = None,
+        # passthrough arguments to _validate_claims
+        # consider putting in options
+        audience: Optional[str] = None,
+        issuer: Optional[str] = None,
+        leeway: Union[int, float, timedelta] = 0,
+        # kwargs
         **kwargs,
     ) -> Dict[str, Any]:
+        if kwargs:
+            warnings.warn(
+                "passing additional kwargs to decode_complete() is deprecated "
+                "and will be removed in pyjwt version 3. "
+                f"Unsupported kwargs: {tuple(kwargs.keys())}",
+                RemovedInPyjwt3Warning,
+            )
         options = dict(options or {})  # shallow-copy or initialize an empty dict
         options.setdefault("verify_signature", True)
 
         # If the user has set the legacy `verify` argument, and it doesn't match
         # what the relevant `options` entry for the argument is, inform the user
         # that they're likely making a mistake.
-        if "verify" in kwargs and kwargs["verify"] != options["verify_signature"]:
+        if verify is not None and verify != options["verify_signature"]:
             warnings.warn(
                 "The `verify` argument to `decode` does nothing in PyJWT 2.0 and newer. "
                 "The equivalent is setting `verify_signature` to False in the `options` dictionary. "
@@ -102,7 +120,7 @@ class PyJWT:
             key=key,
             algorithms=algorithms,
             options=options,
-            **kwargs,
+            detached_payload=detached_payload,
         )
 
         try:
@@ -113,7 +131,9 @@ class PyJWT:
             raise DecodeError("Invalid payload string: must be a json object")
 
         merged_options = {**self.options, **options}
-        self._validate_claims(payload, merged_options, **kwargs)
+        self._validate_claims(
+            payload, merged_options, audience=audience, issuer=issuer, leeway=leeway
+        )
 
         decoded["payload"] = payload
         return decoded
@@ -124,14 +144,39 @@ class PyJWT:
         key: str = "",
         algorithms: Optional[List[str]] = None,
         options: Optional[Dict[str, Any]] = None,
+        # deprecated arg, remove in pyjwt3
+        verify: Optional[bool] = None,
+        # could be used as passthrough to api_jws, consider removal in pyjwt3
+        detached_payload: Optional[bytes] = None,
+        # passthrough arguments to _validate_claims
+        # consider putting in options
+        audience: Optional[str] = None,
+        issuer: Optional[str] = None,
+        leeway: Union[int, float, timedelta] = 0,
+        # kwargs
         **kwargs,
     ) -> Dict[str, Any]:
-        decoded = self.decode_complete(jwt, key, algorithms, options, **kwargs)
+        if kwargs:
+            warnings.warn(
+                "passing additional kwargs to decode() is deprecated "
+                "and will be removed in pyjwt version 3. "
+                f"Unsupported kwargs: {tuple(kwargs.keys())}",
+                RemovedInPyjwt3Warning,
+            )
+        decoded = self.decode_complete(
+            jwt,
+            key,
+            algorithms,
+            options,
+            verify=verify,
+            detached_payload=detached_payload,
+            audience=audience,
+            issuer=issuer,
+            leeway=leeway,
+        )
         return decoded["payload"]
 
-    def _validate_claims(
-        self, payload, options, audience=None, issuer=None, leeway=0, **kwargs
-    ):
+    def _validate_claims(self, payload, options, audience=None, issuer=None, leeway=0):
         if isinstance(leeway, timedelta):
             leeway = leeway.total_seconds()
 
diff --git a/jwt/warnings.py b/jwt/warnings.py
new file mode 100644
index 0000000..8762a8c
--- /dev/null
+++ b/jwt/warnings.py
@@ -0,0 +1,2 @@
+class RemovedInPyjwt3Warning(DeprecationWarning):
+    pass


Test patch:
diff --git a/tests/test_api_jws.py b/tests/test_api_jws.py
index 23975fa..cfbbe21 100644
--- a/tests/test_api_jws.py
+++ b/tests/test_api_jws.py
@@ -12,6 +12,7 @@ from jwt.exceptions import (
     InvalidTokenError,
 )
 from jwt.utils import base64url_decode
+from jwt.warnings import RemovedInPyjwt3Warning
 
 from .utils import crypto_required, key_path, no_crypto_required
 
@@ -770,3 +771,37 @@ class TestJWS:
             'It is required that you pass in a value for the "detached_payload" argument to decode a message having the b64 header set to false.'
             in str(exc.value)
         )
+
+    def test_decode_warns_on_unsupported_kwarg(self, jws, payload):
+        secret = "secret"
+        jws_message = jws.encode(
+            payload, secret, algorithm="HS256", is_payload_detached=True
+        )
+
+        with pytest.warns(RemovedInPyjwt3Warning) as record:
+            jws.decode(
+                jws_message,
+                secret,
+                algorithms=["HS256"],
+                detached_payload=payload,
+                foo="bar",
+            )
+        assert len(record) == 1
+        assert "foo" in str(record[0].message)
+
+    def test_decode_complete_warns_on_unuspported_kwarg(self, jws, payload):
+        secret = "secret"
+        jws_message = jws.encode(
+            payload, secret, algorithm="HS256", is_payload_detached=True
+        )
+
+        with pytest.warns(RemovedInPyjwt3Warning) as record:
+            jws.decode_complete(
+                jws_message,
+                secret,
+                algorithms=["HS256"],
+                detached_payload=payload,
+                foo="bar",
+            )
+        assert len(record) == 1
+        assert "foo" in str(record[0].message)
diff --git a/tests/test_api_jwt.py b/tests/test_api_jwt.py
index 84e41e0..d0443e8 100644
--- a/tests/test_api_jwt.py
+++ b/tests/test_api_jwt.py
@@ -17,6 +17,7 @@ from jwt.exceptions import (
     MissingRequiredClaimError,
 )
 from jwt.utils import base64url_decode
+from jwt.warnings import RemovedInPyjwt3Warning
 
 from .utils import crypto_required, key_path, utc_timestamp
 
@@ -682,3 +683,21 @@ class TestJWT:
         jwt_message = jwt.encode(payload, secret)
         jwt.decode(jwt_message, secret, options=options, algorithms=["HS256"])
         assert options == orig_options
+
+    def test_decode_warns_on_unsupported_kwarg(self, jwt, payload):
+        secret = "secret"
+        jwt_message = jwt.encode(payload, secret)
+
+        with pytest.warns(RemovedInPyjwt3Warning) as record:
+            jwt.decode(jwt_message, secret, algorithms=["HS256"], foo="bar")
+        assert len(record) == 1
+        assert "foo" in str(record[0].message)
+
+    def test_decode_complete_warns_on_unsupported_kwarg(self, jwt, payload):
+        secret = "secret"
+        jwt_message = jwt.encode(payload, secret)
+
+        with pytest.warns(RemovedInPyjwt3Warning) as record:
+            jwt.decode_complete(jwt_message, secret, algorithms=["HS256"], foo="bar")
+        assert len(record) == 1
+        assert "foo" in str(record[0].message)