Bump pyjwt to 2.0.0

The release of pyjwt 2.0.0 changed the behavior of some functions, which caused errors. Fix the errors, use pyjwt 2.0.0's better handling of JWKS, and pin requirement to 2.X to avoid future potential API breaking changes. Change-Id: Ibef736e0f635dfaf4477cc2a90a22665da9f1959
2020-12-23 10:26:08 +01:00
parent bb78d31ced
commit daa51d766f
9 changed files with 76 additions and 77 deletions
--- a/requirements.txt
+++ b/requirements.txt
@@ -24,8 +24,7 @@ alembic
 cryptography>=1.6
 cachecontrol
 cachetools
-# PyJWT 2.0.0 has API breaking changes
-pyjwt<2.0.0
+pyjwt>=2.0.0,<3.0
 iso8601
 psutil
 fb-re2>=1.0.6
--- a/tests/unit/test_auth.py
+++ b/tests/unit/test_auth.py
@@ -17,6 +17,7 @@ import json
 from unittest import mock
 import os.path
 import jwt
+from io import StringIO
 import time

 from zuul.driver import auth
@@ -31,10 +32,10 @@ with open(os.path.join(FIXTURE_DIR,
 algo = jwt.algorithms.RSAAlgorithm(jwt.algorithms.RSAAlgorithm.SHA256)
 with open(os.path.join(FIXTURE_DIR,
                       'auth/oidc-key'), 'r') as k:
-    OIDC_PRIVATE_KEY = algo.prepare_key(k.read().encode('utf-8'))
+    OIDC_PRIVATE_KEY = algo.prepare_key(k.read())
 with open(os.path.join(FIXTURE_DIR,
                       'auth/oidc-key.pub'), 'r') as k:
-    pub_key = algo.prepare_key(k.read().encode('utf-8'))
+    pub_key = algo.prepare_key(k.read())
    pub_jwk = algo.to_jwk(pub_key)
    key = {
        "kid": "OwO",
@@ -52,17 +53,6 @@ with open(os.path.join(FIXTURE_DIR,
    }


-def mock_get(url, params=None, **kwargs):
-    if url == ("https://my.oidc.provider/auth/realms/realm-one/"
-               ".well-known/openid-configuration"):
-        return FakeResponse(FAKE_WELL_KNOWN_CONFIG)
-    elif url == ("https://my.oidc.provider/auth/realms/realm-one/"
-                 "protocol/openid-connect/certs"):
-        return FakeResponse(FAKE_CERTS)
-    else:
-        raise Exception("Unknown URL %s" % url)
-
-
 class FakeResponse:
    def __init__(self, json_dict):
        self._json = json_dict
@@ -71,6 +61,25 @@ class FakeResponse:
        return self._json


+def mock_get(url, params=None, **kwargs):
+    if url == ("https://my.oidc.provider/auth/realms/realm-one/"
+               ".well-known/openid-configuration"):
+        return FakeResponse(FAKE_WELL_KNOWN_CONFIG)
+    else:
+        raise Exception("Unknown URL %s" % url)
+
+
+def mock_urlopen(url, *args, **kwargs):
+    if url == ("https://my.oidc.provider/auth/realms/realm-one/"
+               "protocol/openid-connect/certs"):
+        io = StringIO()
+        json.dump(FAKE_CERTS, io)
+        io.seek(0)
+        return io
+    else:
+        raise Exception("Unknown URL %s" % url)
+
+
 class TestOpenIDConnectAuthenticator(BaseTestCase):
    def test_decodeToken(self):
        """Test the decoding workflow"""
@@ -92,6 +101,9 @@ class TestOpenIDConnectAuthenticator(BaseTestCase):
            algorithm='RS256',
            headers={'kid': 'OwO'})
        with mock.patch('requests.get', side_effect=mock_get):
-            decoded = OIDCAuth.decodeToken(token)
-            for claim in payload.keys():
-                self.assertEqual(payload[claim], decoded[claim])
+            # patching call in PyJWKClient's fetch_data
+            with mock.patch('urllib.request.urlopen',
+                            side_effect=mock_urlopen):
+                decoded = OIDCAuth.decodeToken(token)
+                for claim in payload.keys():
+                    self.assertEqual(payload[claim], decoded[claim])
--- a/tests/unit/test_client.py
+++ b/tests/unit/test_client.py
@@ -136,9 +136,9 @@ class TestWebTokenClient(BaseClientTestCase):
                           key=self.config.get(
                               'auth zuul_operator',
                               'secret'),
-                           algorithm=self.config.get(
+                           algorithms=[self.config.get(
                               'auth zuul_operator',
-                               'driver'),
+                               'driver')],
                           audience=self.config.get(
                               'auth zuul_operator',
                               'client_id'),)
--- a/tests/unit/test_web.py
+++ b/tests/unit/test_web.py
@@ -1351,7 +1351,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='OnlyZuulNoDana',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        resp = self.post_url(
            "api/tenant/tenant-one/project/org/project/autohold",
            headers={'Authorization': 'Bearer %s' % token},
@@ -1386,7 +1386,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
                 },
                 'exp': time.time() - 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        resp = self.post_url(
            "api/tenant/tenant-one/project/org/project/autohold",
            headers={'Authorization': 'Bearer %s' % token},
@@ -1421,7 +1421,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        resp = self.post_url(
            "api/tenant/tenant-one/project/org/project/autohold",
            headers={'Authorization': 'Bearer %s' % token},
@@ -1461,7 +1461,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
                'ref': None,
                'node_hold_expiration': None}
        good_token = jwt.encode(good_authz, key='NoDanaOnlyZuul',
-                                algorithm='HS256').decode('utf-8')
+                                algorithm='HS256')
        req = self.post_url(
            'api/tenant/tenant-one/project/org/project/autohold',
            headers={'Authorization': 'Bearer %s' % good_token},
@@ -1495,7 +1495,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        req = self.post_url(
            'api/tenant/tenant-one/project/org/project/autohold',
            headers={'Authorization': 'Bearer %s' % token},
@@ -1521,7 +1521,7 @@ class TestTenantScopedWebApi(BaseTestWeb):

    def _init_autohold_delete(self, authz):
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')

        client = zuul.rpcclient.RPCClient('127.0.0.1',
                                          self.gearman_server.port)
@@ -1559,7 +1559,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
                     },
                     'exp': time.time() + 3600}
        bad_token = jwt.encode(bad_authz, key='NoDanaOnlyZuul',
-                               algorithm='HS256').decode('utf-8')
+                               algorithm='HS256')
        resp = self.delete_url(
            "api/tenant/tenant-one/autohold/%s" % request_id,
            headers={'Authorization': 'Bearer %s' % bad_token})
@@ -1603,7 +1603,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        path = "api/tenant/%(tenant)s/project/%(project)s/enqueue"
        enqueue_args = {'tenant': 'tenant-one',
                        'project': 'org/project', }
@@ -1655,7 +1655,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        req = self.post_url(path % enqueue_args,
                            headers={'Authorization': 'Bearer %s' % token},
                            json=ref)
@@ -1696,7 +1696,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        path = "api/tenant/%(tenant)s/project/%(project)s/dequeue"
        dequeue_args = {'tenant': 'tenant-one',
                        'project': 'org/project', }
@@ -1818,7 +1818,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
                 'exp': time.time() + 3600,
                 'iat': time.time()}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        req = self.post_url(
            'api/tenant/tenant-one/promote',
            headers={'Authorization': 'Bearer %s' % token},
@@ -1892,7 +1892,7 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        req = self.post_url(
            'api/tenant/tenant-one/project/org/project/autohold',
            headers={'Authorization': 'Bearer %s' % token},
@@ -1915,7 +1915,7 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
                            'project': project, }

            token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                               algorithm='HS256').decode('utf-8')
+                               algorithm='HS256')
            req = self.post_url(path % enqueue_args,
                                headers={'Authorization': 'Bearer %s' % token},
                                json=change)
@@ -1959,7 +1959,7 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
                 'groups': ['ghostbusters', 'secretary'],
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        path = "api/tenant/%(tenant)s/project/%(project)s/enqueue"
        enqueue_args = {'tenant': 'tenant-one',
                        'project': 'org/project2', }
@@ -1985,7 +1985,7 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
                     'car': 'ecto-1'},
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        path = "api/tenant/%(tenant)s/project/%(project)s/enqueue"
        enqueue_args = {'tenant': 'tenant-one',
                        'project': 'org/project', }
@@ -2007,7 +2007,7 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
                 'zuul': {'admin': admin_tenants},
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        # TODO(mhu) deprecated, remove after next release
        req = self.get_url('/api/user/authorizations',
                           headers={'Authorization': 'Bearer %s' % token})
@@ -2049,7 +2049,7 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
            authz = test_user['authz']
            authz['exp'] = time.time() + 3600
            token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                               algorithm='HS256').decode('utf-8')
+                               algorithm='HS256')
            # TODO(mhu) deprecated, remove after next release
            req = self.get_url('/api/user/authorizations',
                               headers={'Authorization': 'Bearer %s' % token})
@@ -2116,7 +2116,7 @@ class TestTenantScopedWebApiTokenWithExpiry(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        resp = self.post_url(
            "api/tenant/tenant-one/project/org/project/autohold",
            headers={'Authorization': 'Bearer %s' % token},
@@ -2152,7 +2152,7 @@ class TestTenantScopedWebApiTokenWithExpiry(BaseTestWeb):
                 'exp': time.time() + 7200,
                 'iat': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        resp = self.post_url(
            "api/tenant/tenant-one/project/org/project/autohold",
            headers={'Authorization': 'Bearer %s' % token},
@@ -2188,7 +2188,7 @@ class TestTenantScopedWebApiTokenWithExpiry(BaseTestWeb):
                 'exp': time.time() + 3600,
                 'iat': time.time()}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        time.sleep(10)
        resp = self.post_url(
            "api/tenant/tenant-one/project/org/project/autohold",
@@ -2232,7 +2232,7 @@ class TestTenantScopedWebApiTokenWithExpiry(BaseTestWeb):
                 'exp': time.time() + 3600,
                 'iat': time.time()}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        req = self.post_url(
            'api/tenant/tenant-one/project/org/project/autohold',
            headers={'Authorization': 'Bearer %s' % token},
@@ -2336,7 +2336,7 @@ class TestCLIViaWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        p = subprocess.Popen(
            [os.path.join(sys.prefix, 'bin/zuul'),
             '--zuul-url', self.base_url, '--auth-token', token,
@@ -2375,7 +2375,7 @@ class TestCLIViaWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        p = subprocess.Popen(
            [os.path.join(sys.prefix, 'bin/zuul'),
             '--zuul-url', self.base_url, '--auth-token', token,
@@ -2404,7 +2404,7 @@ class TestCLIViaWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        p = subprocess.Popen(
            [os.path.join(sys.prefix, 'bin/zuul'),
             '--zuul-url', self.base_url, '--auth-token', token,
@@ -2440,7 +2440,7 @@ class TestCLIViaWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        p = subprocess.Popen(
            [os.path.join(sys.prefix, 'bin/zuul'),
             '--zuul-url', self.base_url, '--auth-token', token,
@@ -2491,7 +2491,7 @@ class TestCLIViaWebApi(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        p = subprocess.Popen(
            [os.path.join(sys.prefix, 'bin/zuul'),
             '--zuul-url', self.base_url, '--auth-token', token,
--- a/tests/zuul_client/test_zuulclient.py
+++ b/tests/zuul_client/test_zuulclient.py
@@ -169,7 +169,7 @@ class TestZuulClientAdmin(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        p = subprocess.Popen(
            ['zuul-client',
             '--zuul-url', self.base_url, '--auth-token', token, '-v',
@@ -209,7 +209,7 @@ class TestZuulClientAdmin(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        p = subprocess.Popen(
            ['zuul-client',
             '--zuul-url', self.base_url, '--auth-token', token, '-v',
@@ -245,7 +245,7 @@ class TestZuulClientAdmin(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        p = subprocess.Popen(
            ['zuul-client',
             '--zuul-url', self.base_url, '--auth-token', token, '-v',
@@ -287,7 +287,7 @@ class TestZuulClientAdmin(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        p = subprocess.Popen(
            ['zuul-client',
             '--zuul-url', self.base_url, '--auth-token', token, '-v',
@@ -338,7 +338,7 @@ class TestZuulClientAdmin(BaseTestWeb):
                 },
                 'exp': time.time() + 3600}
        token = jwt.encode(authz, key='NoDanaOnlyZuul',
-                           algorithm='HS256').decode('utf-8')
+                           algorithm='HS256')
        p = subprocess.Popen(
            ['zuul-client',
             '--zuul-url', self.base_url, '--auth-token', token, '-v',
--- a/zuul/cmd/client.py
+++ b/zuul/cmd/client.py
@@ -581,7 +581,7 @@ class Client(zuul.cmd.ZuulApp):
        try:
            auth_token = jwt.encode(token,
                                    key=key,
-                                    algorithm=driver).decode('utf-8')
+                                    algorithm=driver)
            print("Bearer %s" % auth_token)
            err_code = 0
        except Exception as e:
--- a/zuul/driver/auth/jwt.py
+++ b/zuul/driver/auth/jwt.py
@@ -18,7 +18,6 @@ import math
 import time
 import jwt
 import requests
-import json
 from urllib.parse import urljoin

 from zuul import exceptions
@@ -74,16 +73,16 @@ class JWTAuthenticator(AuthenticatorInterface):
        except jwt.exceptions.InvalidSignatureError:
            raise exceptions.AuthTokenInvalidSignatureException(
                realm=self.realm)
-        except jwt.DecodeError:
+        except jwt.exceptions.DecodeError:
            raise exceptions.AuthTokenUndecodedException(
                realm=self.realm)
        except jwt.exceptions.ExpiredSignatureError:
            raise exceptions.TokenExpiredError(
                realm=self.realm)
-        except jwt.InvalidIssuerError:
+        except jwt.exceptions.InvalidIssuerError:
            raise exceptions.IssuerUnknownError(
                realm=self.realm)
-        except jwt.InvalidAudienceError:
+        except jwt.exceptions.InvalidAudienceError:
            raise exceptions.IncorrectAudienceError(
                realm=self.realm)
        except Exception as e:
@@ -154,7 +153,7 @@ class HS256Authenticator(JWTAuthenticator):
    def _decode(self, rawToken):
        return jwt.decode(rawToken, self.secret, issuer=self.issuer_id,
                          audience=self.audience,
-                          algorithms=self.algorithm)
+                          algorithms=[self.algorithm])


 class RS256Authenticator(JWTAuthenticator):
@@ -172,7 +171,7 @@ class RS256Authenticator(JWTAuthenticator):
    def _decode(self, rawToken):
        return jwt.decode(rawToken, self.public_key, issuer=self.issuer_id,
                          audience=self.audience,
-                          algorithms=self.algorithm)
+                          algorithms=[self.algorithm])


 class OpenIDConnectAuthenticator(JWTAuthenticator):
@@ -203,9 +202,9 @@ class OpenIDConnectAuthenticator(JWTAuthenticator):
            raise exceptions.JWKSException(
                realm=self.realm,
                msg=msg)
-        # TODO keys can probably be cached
+        jwks_client = jwt.PyJWKClient(keys_url)
        try:
-            certs = requests.get(keys_url).json()
+            signing_key = jwks_client.get_signing_key(kid=key_id)
        except Exception as e:
            msg = 'Could not fetch Identity Provider keys at %s: %s'
            logger.error(msg % (keys_url, e))
@@ -213,20 +212,9 @@ class OpenIDConnectAuthenticator(JWTAuthenticator):
                realm=self.realm,
                msg='There was an error while fetching '
                    'keys for Identity Provider, check logs for details')
-        for key_dict in certs['keys']:
-            if key_dict.get('kid') == key_id:
-                # TODO: theoretically two other types of keys are
-                # supported by the JWKS standard. We should raise an error
-                # in the unlikely case 'kty' is not RSA.
-                # (see https://tools.ietf.org/html/rfc7518#section-6.1)
-                key = jwt.algorithms.RSAAlgorithm.from_jwk(
-                    json.dumps(key_dict))
-                algorithm = key_dict.get('alg', None) or self.algorithm
-                return key, algorithm
-        raise exceptions.JWKSException(
-            self.realm,
-            'Cannot verify token: public key %s '
-            'not listed by Identity Provider' % key_id)
+        algorithm = signing_key._jwk_data.get("alg", None) or self.algorithm
+        key = signing_key.key
+        return key, algorithm

    def get_well_known_config(self):
        issuer = self.issuer_id
@@ -258,7 +246,7 @@ class OpenIDConnectAuthenticator(JWTAuthenticator):
        key, algorithm = self.get_key(key_id)
        return jwt.decode(rawToken, key, issuer=self.issuer_id,
                          audience=self.audience,
-                          algorithms=algorithm)
+                          algorithms=[algorithm])


 AUTHENTICATORS = {
--- a/zuul/driver/github/githubconnection.py
+++ b/zuul/driver/github/githubconnection.py
@@ -947,7 +947,7 @@ class GithubClientManager:
        data = {'iat': now, 'exp': expiry, 'iss': self.app_id}
        app_token = jwt.encode(data,
                               self.app_key,
-                               algorithm='RS256').decode('utf-8')
+                               algorithm='RS256')

        headers = {'Accept': PREVIEW_JSON_ACCEPT,
                   'Authorization': 'Bearer %s' % app_token}
--- a/zuul/lib/auth.py
+++ b/zuul/lib/auth.py
@@ -74,7 +74,7 @@ class AuthenticatorRegistry(object):
        cpb.capabilities_registry.register_capabilities('auth', capabilities)

    def authenticate(self, rawToken):
-        unverified = jwt.decode(rawToken, verify=False)
+        unverified = jwt.decode(rawToken, options={'verify_signature': False})
        for auth_name in self.authenticators:
            authenticator = self.authenticators[auth_name]
            if authenticator.issuer_id == unverified.get('iss', ''):