Properly Report Key Utilized in Signing/Encrypting (#103)

* Fixed an issue where the SDK would improperly report the key used when the encryption and signature keys for API differed

Author: lordkyzr

CHANGES.rst

@@ -1,5 +1,9 @@
 CHANGELOG for LaunchKey Python SDK
 ==================================
+3.8.1
+-----
+
+* Fixed an issue where the SDK would improperly report which key was used when the encryption and signature keys differed
 
 3.8.0
 -----

launchkey/__init__.py

@@ -1,5 +1,5 @@
 """LaunchKey Service SDK module"""
-SDK_VERSION = '3.8.0'
+SDK_VERSION = '3.8.1-rc.1'
 LAUNCHKEY_PRODUCTION = "https://api.launchkey.com"
 VALID_JWT_ISSUER_LIST = ["svc", "dir", "org"]
 JOSE_SUPPORTED_JWE_ALGS = ['RSA-OAEP']

launchkey/transports/jose_auth.py

@@ -141,14 +141,19 @@ def __get_kid_from_api_response_headers(headers):
 
         return kid
 
-    def _get_kid_from_api_response(self, response):
+    @staticmethod
+    def _get_kid_from_api_response(response):
         """
         Gets `kid` property from a JWT token within a LaunchKey API
         response.
         :param response: Response object
         :return: string of the `kid`
         """
-        return self.__get_kid_from_api_response_headers(response.headers)
+        try:
+            return response.headers["X-IOV-KEY-ID"]
+        except KeyError:
+            raise UnexpectedAPIResponse("X-IOV-KEY-ID was missing or malformed"
+                                        " in API response.")
 
     @staticmethod
     def parse_api_time(api_time):

tests/test_jose_auth_transport.py

@@ -4,7 +4,7 @@
 from ddt import data, ddt
 from jwkest import JWKESTException
 from jwkest.jws import JWS
-from jwkest.jwt import JWT
+from jwkest.jwt import JWT, BadSyntax
 from mock import MagicMock, ANY, patch, call
 from launchkey.transports import JOSETransport, RequestsTransport
 from launchkey.transports.base import APIResponse, APIErrorResponse
@@ -70,13 +70,15 @@
     "kid": faux_kid
 }
 
+transport_request_headers = {"X-IOV-KEY-ID": faux_kid}
+
 
 class TestJOSETransport3rdParty(unittest.TestCase):
 
     def setUp(self):
         self._transport = JOSETransport()
         self._transport.get = MagicMock(return_value=MagicMock(spec=APIResponse))
-        public_key = APIResponse(valid_private_key, {}, 200)
+        public_key = APIResponse(valid_public_key, transport_request_headers, 200)
         self._transport.get.return_value = public_key
         self._transport._server_time_difference = 0, time()
 
@@ -152,7 +154,7 @@ class TestJWKESTSupportedAlgs(unittest.TestCase):
     def setUp(self):
         self._transport = JOSETransport()
         self._transport.get = MagicMock(return_value=MagicMock(spec=APIResponse))
-        public_key = APIResponse(valid_private_key, {}, 200)
+        public_key = APIResponse(valid_private_key, transport_request_headers, 200)
         self._transport.get.return_value = public_key
         self._transport._server_time_difference = 0, time()
 
@@ -265,7 +267,7 @@ class TestJOSETransportJWTResponse(unittest.TestCase):
     def setUp(self):
         self._transport = JOSETransport()
         self._transport.get = MagicMock(return_value=MagicMock(spec=APIResponse))
-        public_key = APIResponse(valid_private_key, {}, 200)
+        public_key = APIResponse(valid_public_key, transport_request_headers, 200)
         self._transport.get.return_value = public_key
         self._transport._server_time_difference = 0, time()
         self.issuer = "svc"
@@ -421,7 +423,7 @@ class TestJOSETransportJWTRequest(unittest.TestCase):
     def setUp(self):
         self._transport = JOSETransport()
         self._transport.get = MagicMock(return_value=MagicMock(spec=APIResponse))
-        public_key = APIResponse(valid_private_key, {}, 200)
+        public_key = APIResponse(valid_private_key, transport_request_headers, 200)
         self._transport.get.return_value = public_key
         self._transport._server_time_difference = 0, time()
         self.issuer = "svc"
@@ -808,7 +810,7 @@ def setUp(self):
         }
 
         self._requests_transport = MagicMock(spec=RequestsTransport)
-        self._requests_transport.get.return_value = APIResponse(valid_private_key, {}, 200)
+        self._requests_transport.get.return_value = APIResponse(valid_public_key, transport_request_headers, 200)
         self._transport = JOSETransport(http_client=self._requests_transport)
         self._import_rsa_key_patch = patch(
             "launchkey.transports.jose_auth.import_rsa_key",
@@ -850,8 +852,7 @@ def test_key_is_retrieved_by_id_when_key_changed(self):
             "kid": "jwt2keyid"
         }
 
-        self._jwt_patch.return_value.unpack.side_effect = [jwt1, jwt1, jwt2, jwt2]
-
+        self._jwt_patch.return_value.unpack.side_effect = [jwt1, jwt2]
         self._transport.verify_jwt_response(MagicMock(), self.jti, ANY, None)
         self._transport.verify_jwt_response(MagicMock(), self.jti, ANY, None)
         self._requests_transport.get.assert_has_calls([
@@ -865,15 +866,15 @@ def test_key_retrieved_is_used_to_verify_payload(self, rsa_key_patch):
         self._requests_transport.get.return_value.data = valid_public_key
         self._transport.verify_jwt_response(MagicMock(), self.jti, ANY, None)
 
-        # Verify that verify_compact is called one time with key created by our jwkest key patch
-        self._jws_patch.return_value.verify_compact.assert_called_once_with(ANY, keys=[rsa_key_patch.return_value])
-
-        # Assert that the jwkest key patch is built using the import_rsa_key patch return value and the key id
-        # from the header
-        rsa_key_patch.assert_called_with(key=self._import_rsa_key_patch.return_value, kid=faux_kid)
+        # Verify that verify_compact is called one time with key created
+        # by our jwkest key patch
+        self._jws_patch.return_value.verify_compact\
+            .assert_called_once_with(ANY, keys=[rsa_key_patch.return_value])
 
-        # Verify that we used the correct key to retrieve the key id from the header
-        self._requests_transport.get.return_value.headers.get.assert_called_with("X-IOV-JWT")
+        # Assert that the jwkest key patch is built using the import_rsa_key
+        # patch return value and the key id from the header
+        rsa_key_patch.assert_called_with(
+            key=self._import_rsa_key_patch.return_value, kid=faux_kid)
 
     def test_raises_when_kid_header_is_missing(self):
         headers_without_kid = {"alg": "RS512", "typ": "JWT"}
@@ -883,6 +884,31 @@ def test_raises_when_kid_header_is_missing(self):
         with self.assertRaises(JWTValidationFailure):
             self._transport.verify_jwt_response(MagicMock(), self.jti, ANY, None)
 
+    def test_raises_when_jwt_unpack_returns_badsyntax(self):
+        self._jwt_patch.return_value.unpack.side_effect = BadSyntax("test", "error")
+        with self.assertRaises(UnexpectedAPIResponse):
+            self._transport.verify_jwt_response(
+                MagicMock(), self.jti, ANY, None)
+
+    def test_raises_when_jwt_unpack_returns_valueerror(self):
+        self._jwt_patch.return_value.unpack.side_effect = ValueError()
+        with self.assertRaises(UnexpectedAPIResponse):
+            self._transport.verify_jwt_response(
+                MagicMock(), self.jti, ANY, None)
+
+    def test_raises_when_jwt_unpack_returns_indexerror(self):
+        self._jwt_patch.return_value.unpack.side_effect = IndexError()
+        with self.assertRaises(UnexpectedAPIResponse):
+            self._transport.verify_jwt_response(
+                MagicMock(), self.jti, ANY, None)
+
+    def test_raises_when_kid_header_is_missing_from_http_headers(self):
+        self._requests_transport.get.return_value = APIResponse(
+            valid_public_key, {}, 200)
+        with self.assertRaises(UnexpectedAPIResponse):
+            self._transport.verify_jwt_response(
+                MagicMock(), self.jti, ANY, None)
+
     def test_raises_when_kid_header_is_malformed(self):
         headers_with_kid_of_wrong_type = {"alg": "RS512", "typ": "JWT", "kid": 1234}
         jwt = MagicMock()