Merge pull request #350 from davidjb/optional-jwk-alg

Fix JWKS handling with same `kid` but different `alg` across JWKs
mozilla · Jul 31, 2020 · 0e0a5c3 · 0e0a5c3
2 parents 56773c1 + fa137e7
commit 0e0a5c3
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 2 deletions.
diff --git a/HISTORY.rst b/HISTORY.rst
@@ -3,6 +3,12 @@
 History
 -------
 
+1.2.4 (Unreleased)
+==================
+
+* Fix JWKS handling when the same `kid` value is used across JWKs with
+  different `alg` specified
+
 1.2.3 (2020-01-02)
 ===================
 

diff --git a/mozilla_django_oidc/auth.py b/mozilla_django_oidc/auth.py
@@ -158,7 +158,7 @@ def retrieve_matching_jwk(self, token):
             if jwk['kid'] != smart_text(header.kid):
                 continue
             if 'alg' in jwk and jwk['alg'] != smart_text(header.alg):
-                raise SuspiciousOperation('alg values do not match.')
+                continue
             key = jwk
         if key is None:
             raise SuspiciousOperation('Could not find a valid JWKS.')

diff --git a/tests/test_auth.py b/tests/test_auth.py
@@ -961,6 +961,48 @@ def test_retrieve_matching_jwk(self, mock_requests):
         jwk_key = self.backend.retrieve_matching_jwk(force_bytes(token))
         self.assertEqual(jwk_key, get_json_mock.json.return_value['keys'][0])
 
+    @patch('mozilla_django_oidc.auth.requests')
+    def test_retrieve_matching_jwk_same_kid(self, mock_requests):
+        """Test retrieving valid jwk from a list of keys with the same kid"""
+
+        get_json_mock = Mock()
+        get_json_mock.json.return_value = {
+            "keys": [
+                {
+                    "alg": "RS512",
+                    "kid": "foobar",
+                },
+                {
+                    "alg": "RS384",
+                    "kid": "foobar",
+                },
+                {
+                    "alg": "RS256",
+                    "kid": "foobar",
+                }
+            ]
+        }
+        mock_requests.get.return_value = get_json_mock
+
+        header = force_bytes(json.dumps({'alg': 'RS256', 'typ': 'JWT', 'kid': 'foobar'}))
+        payload = force_bytes(json.dumps({'foo': 'bar'}))
+
+        # Compute signature
+        key = b'mysupersecuretestkey'
+        h = hmac.HMAC(key, hashes.SHA256(), backend=default_backend())
+        msg = '{}.{}'.format(smart_text(b64encode(header)), smart_text(b64encode(payload)))
+        h.update(force_bytes(msg))
+        signature = b64encode(h.finalize())
+
+        token = '{}.{}.{}'.format(
+            smart_text(b64encode(header)),
+            smart_text(b64encode(payload)),
+            smart_text(signature)
+        )
+
+        jwk_key = self.backend.retrieve_matching_jwk(force_bytes(token))
+        self.assertEqual(jwk_key, get_json_mock.json.return_value['keys'][2])
+
     @patch('mozilla_django_oidc.auth.requests')
     def test_retrieve_mismatcing_jwk_alg(self, mock_requests):
         """Test retrieving mismatching jwk alg"""
@@ -995,7 +1037,7 @@ def test_retrieve_mismatcing_jwk_alg(self, mock_requests):
         with self.assertRaises(SuspiciousOperation) as ctx:
             self.backend.retrieve_matching_jwk(force_bytes(token))
 
-        self.assertEqual(ctx.exception.args[0], 'alg values do not match.')
+        self.assertEqual(ctx.exception.args[0], 'Could not find a valid JWKS.')
 
     @patch('mozilla_django_oidc.auth.requests')
     def test_retrieve_mismatcing_jwk_kid(self, mock_requests):