Repository: jpadilla/pyjwt

Issue:
PyJWKClient should use "kty" not "alg" parameter
`PyJWKClient` requires keys to have the `alg` parameter, which is an *OPTIONAL* member of a JWK (see [RFC 7517 sec. 4.4](https://tools.ietf.org/html/rfc7517#section-4.4)).  

`PyJWKClient `should instead rely on the `kty` parameter to determine the type of public key.  This parameter "MUST be present in a JWK" (see [RFC 7517 sec. 4.1](https://tools.ietf.org/html/rfc7517#section-4.1)).  This would require decoupling the RSA/EC keys from the hashing algorithm used in the signature, but this is technically correct, as a single RSA key could be used to sign with RS256, RS384, and RS512.

Problems occur with `PyJWKClient` when trying to use keys from Microsoft's Active Directory OpenID keys:
[https://login.microsoftonline.com/common/discovery/keys](https://login.microsoftonline.com/common/discovery/keys).  The keys here are correct according to RFC 7517, but as previously stated, `PyJWKClient` incorrectly relies on optional parameters.

## Expected Result

I would expect `get_signing_key_from_jwt(token)` to be be usable with Microsoft's OpenID keys available above.

## Actual Result

```
PyJWKError                                Traceback (most recent call last)
<ipython-input-71-4b1b09fff47d> in <module>
     19 
     20 jwks_client = PyJWKClient(OPENID_KEYS_URI)
---> 21 k = jwks_client.get_signing_key_from_jwt(token)

.../python3.9/site-packages/jwt/jwks_client.py in get_signing_key_from_jwt(self, token)
     52         unverified = decode_token(token, options={"verify_signature": False})
     53         header = unverified["header"]
---> 54         return self.get_signing_key(header.get("kid"))

.../python3.9/site-packages/jwt/jwks_client.py in get_signing_key(self, kid)
     34 
     35     def get_signing_key(self, kid: str) -> PyJWK:
---> 36         signing_keys = self.get_signing_keys()
     37         signing_key = None
     38 

.../site-packages/jwt/jwks_client.py in get_signing_keys(self)
     21 
     22     def get_signing_keys(self) -> List[PyJWK]:
---> 23         jwk_set = self.get_jwk_set()
     24         signing_keys = []
     25 

.../python3.9/site-packages/jwt/jwks_client.py in get_jwk_set(self)
     18     def get_jwk_set(self) -> PyJWKSet:
     19         data = self.fetch_data()
---> 20         return PyJWKSet.from_dict(data)
     21 
     22     def get_signing_keys(self) -> List[PyJWK]:

.../python3.9/site-packages/jwt/api_jwk.py in from_dict(obj)
     61     def from_dict(obj):
     62         keys = obj.get("keys", [])
---> 63         return PyJWKSet(keys)
     64 
     65     @staticmethod

.../python3.9/site-packages/jwt/api_jwk.py in __init__(self, keys)
     56 
     57         for key in keys:
---> 58             self.keys.append(PyJWK(key))
     59 
     60     @staticmethod

.../python3.9/site-packages/jwt/api_jwk.py in __init__(self, jwk_data, algorithm)
     14 
     15         if not algorithm:
---> 16             raise PyJWKError("Unable to find a algorithm for key: %s" % self._jwk_data)
     17 
     18         self.Algorithm = self._algorithms.get(algorithm)

PyJWKError: Unable to find a algorithm for key: {'kty': 'RSA', 'use': 'sig', 'kid': '5Of9P5F9gCCwCmF2BOHHxDDQ-Dk', 'x5t': '5Of9P5F9gCCwCmF2BOHHxDDQ-Dk', 'n': '2y6laZzXOPwGpMOhh0RcZq-Cng12HRv4EHT_Y6w5WOuNWZxzGFjF77qfTKtp_izFIGlr0IwJnbJsDqmTfAXdDMsfRXpWE6DZ6D0s49coNgu-nEFT7UdkuyfUnfPfU8lZLLzxB4fPp0CpUZIacZWb9Ci83dkqS6yEkppftf8bZOW1Cmz6SQuBbZgDyrm7hKBK8NxmSxJvnqUN6CDdOpxJdLSvIon8EUMcA0VEhNx0acgzZmjedZJEGWO6zs8jrRROkX0_fhpjW1BP4nq5OI6JpXMRgV6LuqCdmg9s3Qvw2k27baa97pxAJprMKwBnHSLcbrjkldREZgQ9NweYbLX-JQ', 'e': 'AQAB', 'x5c': ['MIIDBTCCAe2gAwIBAgIQNmD9my1yu4hPh6X2ySAQMjANBgkqhkiG9w0BAQsFADAtMSswKQYDVQQDEyJhY2NvdW50cy5hY2Nlc3Njb250cm9sLndpbmRvd3MubmV0MB4XDTIwMTExNTE4NTMyMFoXDTI1MTExNDE4NTMyMFowLTErMCkGA1UEAxMiYWNjb3VudHMuYWNjZXNzY29udHJvbC53aW5kb3dzLm5ldDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANsupWmc1zj8BqTDoYdEXGavgp4Ndh0b+BB0/2OsOVjrjVmccxhYxe+6n0yraf4sxSBpa9CMCZ2ybA6pk3wF3QzLH0V6VhOg2eg9LOPXKDYLvpxBU+1HZLsn1J3z31PJWSy88QeHz6dAqVGSGnGVm/QovN3ZKkushJKaX7X/G2TltQps+kkLgW2YA8q5u4SgSvDcZksSb56lDegg3TqcSXS0ryKJ/BFDHANFRITcdGnIM2Zo3nWSRBljus7PI60UTpF9P34aY1tQT+J6uTiOiaVzEYFei7qgnZoPbN0L8NpNu22mve6cQCaazCsAZx0i3G645JXURGYEPTcHmGy1/iUCAwEAAaMhMB8wHQYDVR0OBBYEFEhEsQJa9KyDIqzLnHtB0fPwqAv2MA0GCSqGSIb3DQEBCwUAA4IBAQAl4wkCQOD4UCaiuxVCFzs8PXGxhLJJseuN4TbMxvmmzgBaRPeazEmLqvqUW/VxCAWHiCzK6nVXThpAk9SPxN37HnlLt/wzEfQTapVxTzKj5PTzsEm582dDyw+0WDdNtb37a1BoAEpv6J4zVHTQLprfx4ccCQq4m3v6IDuKHrT/U0TlQz0negsUlDr4iwr3lPCdpnbLbgki8JnP5a/Kh/XMs5qXsJRF2IHqEF+yTupEN+fpNvTItfmGAfoGm+6AIBWhH+SEe6sshDpW7rngVJoLAxCrqDlzrXae1pq1pyvfbeMtHkjY7yl74wdFcDD/J0gOA2u30X/2Ld+5AglZIlBm']}
```

## Reproduction Steps

```python

import jwt
from jwt import PyJWKClient

OPENID_KEYS_URI = 'https://login.microsoftonline.com/common/discovery/keys'
# This token `kid` is not present in the above URI, but a valid token is required by `get_signing_key_from_jwt()`
# I cannot include my own *valid* token, for obvious reasons
# This token is an example from https://docs.microsoft.com/en-us/azure/active-directory/develop/access-tokens
token = "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6Imk2bEdrM0ZaenhSY1ViMkMzbkVRN3N5SEpsWSJ9.eyJhdWQiOiI2ZTc0MTcyYi1iZTU2LTQ4NDMtOWZmNC1lNjZhMzliYjEyZTMiLCJpc3MiOiJodHRwczovL2xvZ2luLm1pY3Jvc29mdG9ubGluZS5jb20vNzJmOTg4YmYtODZmMS00MWFmLTkxYWItMmQ3Y2QwMTFkYjQ3L3YyLjAiLCJpYXQiOjE1MzcyMzEwNDgsIm5iZiI6MTUzNzIzMTA0OCwiZXhwIjoxNTM3MjM0OTQ4LCJhaW8iOiJBWFFBaS84SUFBQUF0QWFaTG8zQ2hNaWY2S09udHRSQjdlQnE0L0RjY1F6amNKR3hQWXkvQzNqRGFOR3hYZDZ3TklJVkdSZ2hOUm53SjFsT2NBbk5aY2p2a295ckZ4Q3R0djMzMTQwUmlvT0ZKNGJDQ0dWdW9DYWcxdU9UVDIyMjIyZ0h3TFBZUS91Zjc5UVgrMEtJaWpkcm1wNjlSY3R6bVE9PSIsImF6cCI6IjZlNzQxNzJiLWJlNTYtNDg0My05ZmY0LWU2NmEzOWJiMTJlMyIsImF6cGFjciI6IjAiLCJuYW1lIjoiQWJlIExpbmNvbG4iLCJvaWQiOiI2OTAyMjJiZS1mZjFhLTRkNTYtYWJkMS03ZTRmN2QzOGU0NzQiLCJwcmVmZXJyZWRfdXNlcm5hbWUiOiJhYmVsaUBtaWNyb3NvZnQuY29tIiwicmgiOiJJIiwic2NwIjoiYWNjZXNzX2FzX3VzZXIiLCJzdWIiOiJIS1pwZmFIeVdhZGVPb3VZbGl0anJJLUtmZlRtMjIyWDVyclYzeERxZktRIiwidGlkIjoiNzJmOTg4YmYtODZmMS00MWFmLTkxYWItMmQ3Y2QwMTFkYjQ3IiwidXRpIjoiZnFpQnFYTFBqMGVRYTgyUy1JWUZBQSIsInZlciI6IjIuMCJ9.pj4N-w_3Us9DrBLfpCt"

jwks_client = PyJWKClient(OPENID_KEYS_URI)
k = jwks_client.get_signing_key_from_jwt(token)
```

Note: the above code will re-produce the exception above, which occurs with a newer, valid token that uses a current `kid`.  I would expect the code above to correctly produce error `PyJWKClientError('Unable to find a signing key that matches: "i6lGk3FZzxRcUb2C3nEQ7syHJlY"')`.

## System Information

    $ python -m jwt.help

```
{
  "cryptography": {
    "version": "3.3.1"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.9.1"
  },
  "platform": {
    "release": "3.10.0-957.27.2.el7.x86_64",
    "system": "Linux"
  },
  "pyjwt": {
    "version": "2.0.0"
  }
}
```

Discussion:
dimaqq: jwt.io tokens are here: https://github.com/jsonwebtoken/jsonwebtoken.github.io/blob/bf9a99c8f37aacbcac0b58b9877179073aea32a1/src/editor/default-tokens.js
the repo is MIT license, so I guess the test vectors can be borrowed?
dajiaji: @dimaqq Thanks for your comments and information.

At the first place, keys in JWKs are independent of hash algorithms, at least in "RSA"(RS*, PS*) and "oct"(HS*).
As someone mentioned in #603, "a single RSA key could be used to sign with RS256, RS384, and RS512"

Therefore, the hash algorithm should not be specified when the JWK is loaded (especially if there is no `alg` parameter), but should be specified with the signature during verification.

However, in the current implementation of pyjwt, the hash algorithm must be specified during the JWK decoding phase due to pyjwt class structure, and this PR is just a workaround to implicitly set the default value of `alg`.

In case of kty=RSA, I adopted RS256 as a default alg value as it is "Recommended" in RFC7518. Again, because it is not possible to determine whether it is RS256, RS384, RS512 from the information contained in JWK (kty=RSA).

Similarly, In case of kty=oct, I adopted HS256 as a default alg value. as it is a unique "Required" algorithm in RFC7518.

On the other hand, in case of kty=EC, `crv` can be used to determine the `alg' value.

> ES256K

At least, we need another PR to support it ([RFC8812](https://tools.ietf.org/html/rfc8812#section-3.1)).
dajiaji: After #623 merged, I'll add support for ES256K, add tests and remove WIP.

Patch:
diff --git a/docs/algorithms.rst b/docs/algorithms.rst
index a686b1d..10402f0 100644
--- a/docs/algorithms.rst
+++ b/docs/algorithms.rst
@@ -8,6 +8,7 @@ This library currently supports:
 * HS384 - HMAC using SHA-384 hash algorithm
 * HS512 - HMAC using SHA-512 hash algorithm
 * ES256 - ECDSA signature algorithm using SHA-256 hash algorithm
+* ES256K - ECDSA signature algorithm with secp256k1 curve using SHA-256 hash algorithm
 * ES384 - ECDSA signature algorithm using SHA-384 hash algorithm
 * ES512 - ECDSA signature algorithm using SHA-512 hash algorithm
 * RS256 - RSASSA-PKCS1-v1_5 signature algorithm using SHA-256 hash algorithm
@@ -16,7 +17,7 @@ This library currently supports:
 * PS256 - RSASSA-PSS signature using SHA-256 and MGF1 padding with SHA-256
 * PS384 - RSASSA-PSS signature using SHA-384 and MGF1 padding with SHA-384
 * PS512 - RSASSA-PSS signature using SHA-512 and MGF1 padding with SHA-512
-* EdDSA - Ed25519255 signature using SHA-512. Provides 128-bit security
+* EdDSA - Ed25519 signature using SHA-512. Provides 128-bit security
 
 Asymmetric (Public-key) Algorithms
 ----------------------------------
diff --git a/jwt/api_jwk.py b/jwt/api_jwk.py
index 3c10e07..a0f6364 100644
--- a/jwt/api_jwk.py
+++ b/jwt/api_jwk.py
@@ -1,7 +1,7 @@
 import json
 
 from .algorithms import get_default_algorithms
-from .exceptions import PyJWKError, PyJWKSetError
+from .exceptions import InvalidKeyError, PyJWKError, PyJWKSetError
 
 
 class PyJWK:
@@ -9,11 +9,40 @@ class PyJWK:
         self._algorithms = get_default_algorithms()
         self._jwk_data = jwk_data
 
+        kty = self._jwk_data.get("kty", None)
+        if not kty:
+            raise InvalidKeyError("kty is not found: %s" % self._jwk_data)
+
         if not algorithm and isinstance(self._jwk_data, dict):
             algorithm = self._jwk_data.get("alg", None)
 
         if not algorithm:
-            raise PyJWKError("Unable to find a algorithm for key: %s" % self._jwk_data)
+            # Determine alg with kty (and crv).
+            crv = self._jwk_data.get("crv", None)
+            if kty == "EC":
+                if crv == "P-256" or not crv:
+                    algorithm = "ES256"
+                elif crv == "P-384":
+                    algorithm = "ES384"
+                elif crv == "P-521":
+                    algorithm = "ES512"
+                elif crv == "secp256k1":
+                    algorithm = "ES256K"
+                else:
+                    raise InvalidKeyError("Unsupported crv: %s" % crv)
+            elif kty == "RSA":
+                algorithm = "RS256"
+            elif kty == "oct":
+                algorithm = "HS256"
+            elif kty == "OKP":
+                if not crv:
+                    raise InvalidKeyError("crv is not found: %s" % self._jwk_data)
+                if crv == "Ed25519":
+                    algorithm = "EdDSA"
+                else:
+                    raise InvalidKeyError("Unsupported crv: %s" % crv)
+            else:
+                raise InvalidKeyError("Unsupported kty: %s" % kty)
 
         self.Algorithm = self._algorithms.get(algorithm)
 


Test patch:
diff --git a/tests/test_api_jwk.py b/tests/test_api_jwk.py
index 102af87..e0787f4 100644
--- a/tests/test_api_jwk.py
+++ b/tests/test_api_jwk.py
@@ -1,12 +1,20 @@
 import json
 
+import pytest
+
 from jwt.algorithms import has_crypto
 from jwt.api_jwk import PyJWK, PyJWKSet
+from jwt.exceptions import InvalidKeyError, PyJWKError
 
 from .utils import crypto_required, key_path
 
 if has_crypto:
-    from jwt.algorithms import RSAAlgorithm
+    from jwt.algorithms import (
+        ECAlgorithm,
+        Ed25519Algorithm,
+        HMACAlgorithm,
+        RSAAlgorithm,
+    )
 
 
 class TestPyJWK:
@@ -52,6 +60,158 @@ class TestPyJWK:
         assert jwk.key_id == "keyid-abc123"
         assert jwk.public_key_use == "sig"
 
+    @crypto_required
+    def test_should_load_key_without_alg_from_dict(self):
+
+        with open(key_path("jwk_rsa_pub.json")) as keyfile:
+            key_data = json.loads(keyfile.read())
+
+        jwk = PyJWK.from_dict(key_data)
+
+        assert jwk.key_type == "RSA"
+        assert isinstance(jwk.Algorithm, RSAAlgorithm)
+        assert jwk.Algorithm.hash_alg == RSAAlgorithm.SHA256
+
+    @crypto_required
+    def test_should_load_key_from_dict_with_algorithm(self):
+
+        with open(key_path("jwk_rsa_pub.json")) as keyfile:
+            key_data = json.loads(keyfile.read())
+
+        jwk = PyJWK.from_dict(key_data, algorithm="RS256")
+
+        assert jwk.key_type == "RSA"
+        assert isinstance(jwk.Algorithm, RSAAlgorithm)
+        assert jwk.Algorithm.hash_alg == RSAAlgorithm.SHA256
+
+    @crypto_required
+    def test_should_load_key_ec_p256_from_dict(self):
+
+        with open(key_path("jwk_ec_pub_P-256.json")) as keyfile:
+            key_data = json.loads(keyfile.read())
+
+        jwk = PyJWK.from_dict(key_data)
+
+        assert jwk.key_type == "EC"
+        assert isinstance(jwk.Algorithm, ECAlgorithm)
+        assert jwk.Algorithm.hash_alg == ECAlgorithm.SHA256
+
+    @crypto_required
+    def test_should_load_key_ec_p384_from_dict(self):
+
+        with open(key_path("jwk_ec_pub_P-384.json")) as keyfile:
+            key_data = json.loads(keyfile.read())
+
+        jwk = PyJWK.from_dict(key_data)
+
+        assert jwk.key_type == "EC"
+        assert isinstance(jwk.Algorithm, ECAlgorithm)
+        assert jwk.Algorithm.hash_alg == ECAlgorithm.SHA384
+
+    @crypto_required
+    def test_should_load_key_ec_p521_from_dict(self):
+
+        with open(key_path("jwk_ec_pub_P-521.json")) as keyfile:
+            key_data = json.loads(keyfile.read())
+
+        jwk = PyJWK.from_dict(key_data)
+
+        assert jwk.key_type == "EC"
+        assert isinstance(jwk.Algorithm, ECAlgorithm)
+        assert jwk.Algorithm.hash_alg == ECAlgorithm.SHA512
+
+    @crypto_required
+    def test_should_load_key_ec_secp256k1_from_dict(self):
+
+        with open(key_path("jwk_ec_pub_secp256k1.json")) as keyfile:
+            key_data = json.loads(keyfile.read())
+
+        jwk = PyJWK.from_dict(key_data)
+
+        assert jwk.key_type == "EC"
+        assert isinstance(jwk.Algorithm, ECAlgorithm)
+        assert jwk.Algorithm.hash_alg == ECAlgorithm.SHA256
+
+    @crypto_required
+    def test_should_load_key_hmac_from_dict(self):
+
+        with open(key_path("jwk_hmac.json")) as keyfile:
+            key_data = json.loads(keyfile.read())
+
+        jwk = PyJWK.from_dict(key_data)
+
+        assert jwk.key_type == "oct"
+        assert isinstance(jwk.Algorithm, HMACAlgorithm)
+        assert jwk.Algorithm.hash_alg == HMACAlgorithm.SHA256
+
+    @crypto_required
+    def test_should_load_key_hmac_without_alg_from_dict(self):
+
+        with open(key_path("jwk_hmac.json")) as keyfile:
+            key_data = json.loads(keyfile.read())
+
+        del key_data["alg"]
+        jwk = PyJWK.from_dict(key_data)
+
+        assert jwk.key_type == "oct"
+        assert isinstance(jwk.Algorithm, HMACAlgorithm)
+        assert jwk.Algorithm.hash_alg == HMACAlgorithm.SHA256
+
+    @crypto_required
+    def test_should_load_key_okp_without_alg_from_dict(self):
+
+        with open(key_path("jwk_okp_pub_Ed25519.json")) as keyfile:
+            key_data = json.loads(keyfile.read())
+
+        jwk = PyJWK.from_dict(key_data)
+
+        assert jwk.key_type == "OKP"
+        assert isinstance(jwk.Algorithm, Ed25519Algorithm)
+
+    @crypto_required
+    def test_from_dict_should_throw_exception_if_arg_is_invalid(self):
+
+        with open(key_path("jwk_rsa_pub.json")) as keyfile:
+            valid_rsa_pub = json.loads(keyfile.read())
+        with open(key_path("jwk_ec_pub_P-256.json")) as keyfile:
+            valid_ec_pub = json.loads(keyfile.read())
+        with open(key_path("jwk_okp_pub_Ed25519.json")) as keyfile:
+            valid_okp_pub = json.loads(keyfile.read())
+
+        # Unknown algorithm
+        with pytest.raises(PyJWKError):
+            PyJWK.from_dict(valid_rsa_pub, algorithm="unknown")
+
+        # Missing kty
+        v = valid_rsa_pub.copy()
+        del v["kty"]
+        with pytest.raises(InvalidKeyError):
+            PyJWK.from_dict(v)
+
+        # Unknown kty
+        v = valid_rsa_pub.copy()
+        v["kty"] = "unknown"
+        with pytest.raises(InvalidKeyError):
+            PyJWK.from_dict(v)
+
+        # Unknown EC crv
+        v = valid_ec_pub.copy()
+        v["crv"] = "unknown"
+        with pytest.raises(InvalidKeyError):
+            PyJWK.from_dict(v)
+
+        # Unknown OKP crv
+        v = valid_okp_pub.copy()
+        v["crv"] = "unknown"
+        with pytest.raises(InvalidKeyError):
+            PyJWK.from_dict(v)
+
+        # Missing OKP crv
+        v = valid_okp_pub.copy()
+        del v["crv"]
+        with pytest.raises(InvalidKeyError):
+            PyJWK.from_dict(v)
+
 
 class TestPyJWKSet:
     @crypto_required