Verify the jwt token in the PyFxA lib instead of calling out to the f… (

#80)

Verify the jwt token in the PyFxA lib instead of calling out to the fxa /verify service to do it.

Co-authored-by: Ryan Kelly <rfkelly@mozilla.com>

fxa/oauth.py

@@ -9,9 +9,10 @@
 from six import string_types
 from six.moves.urllib.parse import urlparse, urlunparse, urlencode, parse_qs
 
+import jwt
 from fxa.cache import MemoryCache, DEFAULT_CACHE_EXPIRY
 from fxa.constants import PRODUCTION_URLS
-from fxa.errors import OutOfProtocolError, ScopeMismatchError
+from fxa.errors import OutOfProtocolError, ScopeMismatchError, TrustError
 from fxa._utils import APIClient, scope_matches, get_hmac
 
 
@@ -222,6 +223,26 @@ def authorize_token(self, sessionOrAssertion, scope=None, client_id=None):
 
         return resp['access_token']
 
+    def _verify_jwt_token(self, key, token):
+        pubkey = jwt.algorithms.RSAAlgorithm.from_jwk(key)
+        # The FxA OAuth ecosystem currently doesn't make good use of aud, and
+        # instead relies on scope for restricting which services can accept
+        # which tokens. So there's no value in checking it here, and in fact if
+        # we check it here, it fails because the right audience isn't being
+        # requested.
+        decoded = jwt.decode(
+            token, pubkey, algorithms=['RS256'], options={'verify_aud': False}
+        )
+        if jwt.get_unverified_header(token).get('typ') != 'at+jwt':
+            raise TrustError
+        return {
+            'user': decoded.get('sub'),
+            'client_id': decoded.get('client_id'),
+            'scope': decoded.get('scope'),
+            'generation': decoded.get('fxa-generation'),
+            'profile_changed_at': decoded.get('fxa-profileChangedAt')
+        }
+
     def verify_token(self, token, scope=None):
         """Verify an OAuth token, and retrieve user id and scopes.
 
@@ -239,14 +260,45 @@ def verify_token(self, token, scope=None):
             resp = None
 
         if resp is None:
-            url = '/verify'
-            body = {
-                'token': token
-            }
-            resp = self.apiclient.post(url, body)
-
+            # We want to fetch
+            # https://oauth.accounts.firefox.com/.well-known/openid-configuration
+            # and then get the jwks_uri key to get the /jwks url, but we'll
+            # just hardcodes it like this for now; our /jwks url will never
+            # change.
+            # https://github.com/mozilla/PyFxA/issues/81 is an issue about
+            # getting the jwks url out of the openid-configuration.
+
+            keys = self.apiclient.get('/jwks').get('keys', [])
+            resp = None
+            try:
+                for k in keys:
+                    try:
+                        resp = self._verify_jwt_token(json.dumps(k), token)
+                        break
+                    except jwt.exceptions.InvalidSignatureError:
+                        # It's only worth trying other keys in the event of
+                        # `InvalidSignature`; if it was invalid for other reasons
+                        # (e.g. it's expired) then using a different key won't
+                        # help.
+                        continue
+                else:
+                    # It's a well-formed JWT, but not signed by any of the advertized keys.
+                    # We can immediately surface this as an error.
+                    if len(keys) > 0:
+                        raise TrustError({"error": "invalid signature"})
+            except (jwt.exceptions.DecodeError, jwt.exceptions.InvalidKeyError):
+                # It wasn't a JWT at all, or it was signed using a key type we
+                # don't support. Fall back to asking the FxA server to verify.
+                pass
+            except jwt.exceptions.PyJWTError as e:
+                # Any other JWT-related failure (e.g. expired token) can
+                # immediately surface as a trust error.
+                raise TrustError({"error": str(e)})
+            if resp is None:
+                resp = self.apiclient.post('/verify', {'token': token})
             missing_attrs = ", ".join([
-                k for k in ('user', 'scope', 'client_id') if k not in resp
+                k for k in ('user', 'scope', 'client_id')
+                if resp.get(k) is None
             ])
             if missing_attrs:
                 error_msg = '{0} missing in OAuth response'.format(

fxa/tests/bad-key.json

@@ -0,0 +1,15 @@
+{
+  "kty": "RSA",
+  "n": "2dGtl00RvX80bh60fGQm672QADWRsFSlAV6TBppLhUIUz3XekTMW8IqPCN25swmaEqUKyEjtb0LAc_a0euQ5l2PosPeZyLHtwAxTC4Et-X4n-Vc3RIbKhEnktzmEAAefxbdM3YlGV0Q-a-GSnD3dskfO91KWtdyOiEoOX0aO69QuCdYr-cw7BXHn1nicwCCEvt6T5pjZE-bYuZWBaeeShGhzjPFu9fkFe1Mh-bflaRx08d3AyvPjXYQ-Vz-sRj09sVUzHifmCP1LmdJPyRGHZWFaG_E54cNgSPS9KTvIXJdi5jKunRWUdz_QMn-1PS9aG7ICekxPuXtIkTsN-FuMDQ",
+  "e": "AQAB",
+  "d": "ypjRioI_tu8DOE3M-Eo7PVezAc7RtJ32YAC0ZhbLmaXuL0zl-E35z0BBbDC3kcSjjyX9km7qkWsYEIRuOEPhIWPnQfr7CgYdKl0MPQ4pUf86dRvfJxjscwE8AAQ6N8DfpgopL4GIcJDkMnm8YSDV_QX3hmlGDt-Xn0KqfYnmU4fu_AMnNALNhpJPsZEj0vUZmhuRZx1d8kiTYnTUn-tw3emJTf9sCbUztN447iOvMORPWhSgxeiTJmmLF8B7uroyWEa_PAbi5ICZD8VErqhbXcDp8vp6JW0IwNZFCM_3FhHArnjJhwu693_bs9js5tItiAc5vlG7O1aBnudSqcU9gQ",
+  "p": "72tDFyrfg2s2oDuDhdoArK1O1lWdbAl1pF93z54-N8QnfsSgSredMJSr9YLkxIas5S7Dwrt2cgmZxcgYrNB5VAfBI6sEO7aitBGl_FQ8lnfuWXuJySz79ReOFWsxFh7yvsHaLc7-AU5zs3Eth-tUHYRlturTkIRCWZA1GXczie0",
+  "q": "6Od2sMaABeIJ1N_s3WV_MgiG-lH85UQ8bJ6E_mYXit2BzGEaP1S7ziOjC9bqQTUA7acbRiEFsEKUthiL9Mq83Ykz33YAxCxdnhYJeaLQXyxEUwD4QeJjAxEM_hmdp_jw81A9I1VbykvICaxMiKFbJRjdCcQUPwd0ks6BajrHRqE",
+  "dp": "Dewb26YHl1nNtGWhkVALB_-P_RC_db_bEnLmwbD_Bzkl3s5KT-37ew66LS2uZes69JzSj2ldSuZaBdFL9gitdYB577wLI_nL3lLEZzbyywEwYA26BnPSNv9mqMIx-YpVLViSemV2CddpDP9A2Y2Kt6iyTX-8S8QVI6YqIE_5NEU",
+  "dq": "pDd336LGZ9998SPGPkCvU5bhnSQr_XeEZ03KFhnj5ZzTRUj-GcNj_C_yaYmqeDmoIukdePAVs7buZgqevAYq7sUr1xa76ZYimDGPkr0y0LhEoIXhhYrbJzFhiNSE7ge_1L8xrHUmGFggfnmHKRskSd7jE1y2rc5S6qRCaaVyFqE",
+  "qi": "CMdqnM8YxiInZGEXZFXlU8f82Ft3fN4KbkhIg8O1b_aC1M6RQvadEQTpnJdo7dOJOEZaM24AtC5rewxOPA7VU-B3icfV2_kZdD474Q8hqjD1uQbs-DbfCnxg_2AftrYoZ4OKfMmeMAMnF6R9sZauJQUIafVOd8nSPkMPNY28s28",
+  "kid": "20200604-d56c4825",
+  "alg": "RS256",
+  "use": "sig",
+  "fxa-createdAt": 1591304400
+}

fxa/tests/jwks.json

@@ -0,0 +1,16 @@
+{
+    "keys": [
+        {
+            "kty": "RSA",
+            "n": "nW_losfifTdqolJzRvQEHYLzjf25eX7MriczYrUnbr25runIyz214WAuTeAECDpXGJo__J6brUugkLFaf_NGv-JpJ44QKUiZKcw7qB1N3sEy2WF3XbUR0W0w28pfA2WbwcTRb1j0mj0KPWltCFCK51_KeINMuCTDC9UyXUZjwpSQyJ6lYQVK_n2XR8K2qohOE8I3k03dRkZmZ_D6DLHUUD7hp6pdUpvp2Q6pl_AI59s1J3Z-tCgy_N7ja9QdXE8K6hFAjoF3p5ix46vo6M6HeUGVkVrjEa-Lh15dFkmf6_-8N0r9owwNxpNqkT2nzVdZY2LwLzzqqmgzfP0lbhziaw",
+            "e": "AQAB",
+            "dp": "aod_c9v-N82vmOppJQkIUjSOf_pkmrxJZZ9eJO-ebJd5OsxN_GLOFHa3AH0-vlUoiwFOsziB9yq33EkQT0r9BYcwXEvHJKX5smt17wmIskakLw2FWozSwNf9bgCPoIBh2NyVtcJ0p1SaO3IuIuQsQetfmwkqHbdKOYUnuNc0IuE",
+            "dq": "muc3N3YzJ87RLiBij6xfAliSxdMDg6zKBFXwPRHQJJ0cg6lbvnpnp8XJjjhmYov_2xmICi3C_LO6fwe8KyUOyiPkb0VbjWZtq4Iol9qkQ0iKTnGXkoTfBHVheGq5QoAhxiX7xExd4Gnog5KocrexFWuiZQ0Ul22Bji3gqJhwvcE",
+            "qi": "xguY_G6Ld0Rp7a_ZHAFnAr3Q5Dzhjhkp3vgCi1uNp2jmP3QYng-GvP2xaLcLA0HLBOc0ghgSJYcnmmOB6bxVkVc5R0Hg17-tLlOgQejCd5mQUeMmp_upAScPHzoEea-OM9O_mHtM5BuuroaLIJdhxYolRkKfwD35cwdMX2j9H_4",
+            "kid": "20191118-e43b24c6",
+            "alg": "RS256",
+            "use": "sig",
+            "fxa-createdAt": 1574056800
+        }
+    ]
+}

fxa/tests/private-key.json

@@ -0,0 +1,15 @@
+{
+    "kty": "RSA",
+    "n": "nW_losfifTdqolJzRvQEHYLzjf25eX7MriczYrUnbr25runIyz214WAuTeAECDpXGJo__J6brUugkLFaf_NGv-JpJ44QKUiZKcw7qB1N3sEy2WF3XbUR0W0w28pfA2WbwcTRb1j0mj0KPWltCFCK51_KeINMuCTDC9UyXUZjwpSQyJ6lYQVK_n2XR8K2qohOE8I3k03dRkZmZ_D6DLHUUD7hp6pdUpvp2Q6pl_AI59s1J3Z-tCgy_N7ja9QdXE8K6hFAjoF3p5ix46vo6M6HeUGVkVrjEa-Lh15dFkmf6_-8N0r9owwNxpNqkT2nzVdZY2LwLzzqqmgzfP0lbhziaw",
+    "e": "AQAB",
+    "d": "A0T8ntnJ0VHiTAasUC_nGfnLNwqq3GQOuhskTQt4CyuzfHdsbRQV_90wePuK-eTEREWHyKY-k2W7quWT_I4_zOJVzrV7jm_shlqwbnhVUqfud2c3xGFrQk8jMZ1NRdPJXUfKFabiRYW4_bd_xSlvCQkLOl93q2dQCaHezuMUVUkaNT18ALNbmWjPzv0Umf82XGbkWR8d0ta3BIrtnUZywG-xDf6U_FnK57cdpQyxFLtfF1xJG1pZ-4WdwJREPqhedU5LUfEJ6hjsDi_BvJiNyigyqtIU_j5HXWT_wfbRZ6O1kUWS08D07YcVkbcnyDe4v0nOJeOjw4UnvNmCixL3cQ",
+    "p": "zjDD7yaijNZKoIpnXRJWRnDPAwAvlFg3qPgDqV6XugDzhvPCAj5wQOAe5senLPBqshR049aq_J44Pi9VYFikZn4Ijfzw-Q2BeEBrIvx1q72EHFHO4yKbXdy5fYUja-8z_uZAy-TcW6SuMf5RgsEevesGAZQTC0p8bCQt1PzNPTk",
+    "q": "w3gfsp6S1tdtm-0ngyY-YH2oxBcTMo4G3e32UHQoBFL5EEaA5_ng7_o_0pUoq4vDTtVtfBXU_ZaXW6dUo0JgIlHvX_MeiOvxk7c6BB3zNqN6WyYzcOtEB1sy25-2Z3CkY2jm0D9C8Q3ESSoEIKa9-YRMG_608QaXFiicRBtBQMM",
+    "dp": "aod_c9v-N82vmOppJQkIUjSOf_pkmrxJZZ9eJO-ebJd5OsxN_GLOFHa3AH0-vlUoiwFOsziB9yq33EkQT0r9BYcwXEvHJKX5smt17wmIskakLw2FWozSwNf9bgCPoIBh2NyVtcJ0p1SaO3IuIuQsQetfmwkqHbdKOYUnuNc0IuE",
+    "dq": "muc3N3YzJ87RLiBij6xfAliSxdMDg6zKBFXwPRHQJJ0cg6lbvnpnp8XJjjhmYov_2xmICi3C_LO6fwe8KyUOyiPkb0VbjWZtq4Iol9qkQ0iKTnGXkoTfBHVheGq5QoAhxiX7xExd4Gnog5KocrexFWuiZQ0Ul22Bji3gqJhwvcE",
+    "qi": "xguY_G6Ld0Rp7a_ZHAFnAr3Q5Dzhjhkp3vgCi1uNp2jmP3QYng-GvP2xaLcLA0HLBOc0ghgSJYcnmmOB6bxVkVc5R0Hg17-tLlOgQejCd5mQUeMmp_upAScPHzoEea-OM9O_mHtM5BuuroaLIJdhxYolRkKfwD35cwdMX2j9H_4",
+    "kid": "20191118-e43b24c6",
+    "alg": "RS256",
+    "use": "sig",
+    "fxa-createdAt": 1574056800
+}

fxa/tests/test_oauth.py

@@ -2,7 +2,9 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this file,
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 
+import os
 import json
+import jwt
 import responses
 import six
 try:
@@ -22,6 +24,13 @@
 TEST_SERVER_URL = "https://server/v1"
 
 
+def add_jwks_response():
+    responses.add(responses.GET,
+                  'https://server/v1/jwks',
+                  body=open(os.path.join(os.path.dirname(__file__), "jwks.json")).read(),
+                  content_type='application/json')
+
+
 class TestClientServerUrl(unittest.TestCase):
     def test_trailing_slash_without_prefix_added_prefix(self):
         client = Client('abc', 'cake', "https://server/")
@@ -165,9 +174,9 @@ def setUp(self):
                       'https://server/v1/verify',
                       body=body,
                       content_type='application/json')
-
+        add_jwks_response()
         self.verification = self.client.verify_token(token='abc')
-        self.response = responses.calls[0]
+        self.response = responses.calls[1]
 
     def test_reaches_server_on_verify_url(self):
         self.assertEqual(self.response.request.url,
@@ -194,6 +203,7 @@ def test_raises_error_if_some_attributes_are_not_returned(self):
                       'https://server/v1/verify',
                       body='{"missing": "attributes"}',
                       content_type='application/json')
+        add_jwks_response()
         self.assertRaises(fxa.errors.OutOfProtocolError,
                           self.client.verify_token,
                           token='1234')
@@ -205,6 +215,7 @@ def test_raises_error_if_scopes_do_not_match(self):
                       'https://server/v1/verify',
                       body=body,
                       content_type='application/json')
+        add_jwks_response()
         self.assertRaises(fxa.errors.ScopeMismatchError,
                           self.client.verify_token,
                           token='1234',
@@ -372,6 +383,7 @@ def setUp(self):
                       'https://server/v1/authorization',
                       body='{"access_token": "izatoken"}',
                       content_type='application/json')
+        add_jwks_response()
 
     @responses.activate
     def test_authorize_token_with_default_arguments(self):
@@ -539,6 +551,7 @@ def setUp(self):
                       'https://server/v1/verify',
                       body=self.body,
                       content_type='application/json')
+        add_jwks_response()
 
     def test_has_default_cache(self):
         self.assertIsNotNone(self.client.cache)
@@ -602,6 +615,84 @@ def test_monkey_patch_for_gevent(self):
         fxa._utils.requests = old_requests
 
 
+class TestJwtToken(unittest.TestCase):
+
+    server_url = TEST_SERVER_URL
+
+    def callback(self, request):
+        if self.verify_will_succeed:
+            return (200, {}, self.body)
+        return (500, {}, '{}')
+
+    def _make_jwt(self, payload, key, alg="RS256", header={"typ": "at+jwt"}):
+        header = header.copy()  # So we don't accidentally mutate argument in-place
+        return six.ensure_text(jwt.encode(payload, key, alg, header))
+
+    def setUp(self):
+        self.client = Client(server_url=self.server_url)
+        self.body = ('{"user": "alice", "scope": ["profile"],'
+                     '"client_id": "abc"}')
+        responses.add_callback(responses.POST,
+                               'https://server/v1/verify',
+                               callback=self.callback,
+                               content_type='application/json')
+        add_jwks_response()
+        self.verify_will_succeed = True
+
+    def get_file_contents(self, filename):
+        return jwt.algorithms.RSAAlgorithm.from_jwk(open(
+            os.path.join(
+                os.path.dirname(__file__),
+                filename
+            )
+        ).read())
+
+    @responses.activate
+    def test_good_jwt_token(self):
+        private_key = self.get_file_contents("private-key.json")
+        token = self._make_jwt({
+            "sub": "asdf",
+            "scope": "qwer",
+            "client_id": "foo"
+        }, private_key)
+        self.client.verify_token(token)
+        for c in responses.calls:
+            if c.request.url == 'https://server/v1/verify':
+                raise Exception("testing with a good token should not have \
+                                 resulted in a call to /verify, but it did.")
+
+    @responses.activate
+    def test_wrong_key_jwt_token(self):
+        self.verify_will_succeed = False
+        bad_key = self.get_file_contents("bad-key.json")
+        token = self._make_jwt({}, bad_key)
+        with self.assertRaises(fxa.errors.TrustError):
+            self.client.verify_token(token)
+        for c in responses.calls:
+            if c.request.url == 'https://server/v1/verify':
+                raise Exception("testing with a well-formed token with invalid signature \
+                                 should not have resulted in a call to /verify, but it did.")
+
+    @responses.activate
+    def test_expired_jwt_token(self):
+        private_key = self.get_file_contents("private-key.json")
+        token = self._make_jwt({"qwer": "asdf", "exp": 0}, private_key)
+        with self.assertRaises(fxa.errors.TrustError):
+            self.client.verify_token(token)
+
+    @responses.activate
+    def test_garbage_jwt_token(self):
+        self.verify_will_succeed = False
+        with self.assertRaises(fxa.errors.ServerError):
+            self.client.verify_token("garbage")
+        for c in responses.calls:
+            if c.request.url == 'https://server/v1/verify':
+                break
+        else:
+            raise Exception("testing with a garbage token should have \
+                             called /verify, but it did not.")
+
+
 class AnyStringValue:
 
     def __eq__(self, other):

setup.py

@@ -28,8 +28,9 @@ def open_file(filename):
     "requests>=2.4.2",
     "cryptography",
     "PyBrowserID",
+    "PyJWT",
     "hawkauthlib",
-    "six"
+    "six>=1.14"
 ]
 
 if sys.version_info < (2, 7, 9):