Sem-Ver: api-break Share request sessions across key retriever instances so as to use a common cache.

This change makes it so that when instantiating the HTTPSPublicKeyRetriever class
    multiple times, they all share the same requests session with cache.

    Previously, each instance would create its own session with isolated cache. Since
    the library by default always creates a new instance of the ASAP_KEY_RETRIEVER_CLASS
    class (e.g. common.backend.Backend#get_verifier), cached responses were never reused.

Author: dbaxa

atlassian_jwt_auth/contrib/aiohttp/key.py

@@ -11,6 +11,7 @@
 
 class HTTPSPublicKeyRetriever(_HTTPSPublicKeyRetriever):
     """A class for retrieving JWT public keys with aiohttp"""
+    _class_session = None
 
     def __init__(self, base_url, *, loop=None):
         if loop is None:
@@ -19,7 +20,10 @@ def __init__(self, base_url, *, loop=None):
         super().__init__(base_url)
 
     def _get_session(self):
-        return aiohttp.ClientSession(loop=self.loop)
+        if HTTPSPublicKeyRetriever._class_session is None:
+            HTTPSPublicKeyRetriever._class_session = aiohttp.ClientSession(
+                loop=self.loop)
+        return HTTPSPublicKeyRetriever._class_session
 
     async def _retrieve(self, url, requests_kwargs):
         try:

atlassian_jwt_auth/key.py

@@ -72,6 +72,9 @@ class HTTPSPublicKeyRetriever(BasePublicKeyRetriever):
     """ This class retrieves public key from a https location based upon the
          given key id.
     """
+    # Use a static requests session, reused/shared by all instances of
+    # HTTPSPublicKeyRetriever:
+    _class_session = None
 
     def __init__(self, base_url):
         if base_url is None or not base_url.startswith('https://'):
@@ -83,9 +86,10 @@ def __init__(self, base_url):
         self._session = self._get_session()
 
     def _get_session(self):
-        session = requests.Session()
-        session.mount('https://', cachecontrol.CacheControlAdapter())
-        return session
+        if HTTPSPublicKeyRetriever._class_session is None:
+            session = cachecontrol.CacheControl(requests.Session())
+            HTTPSPublicKeyRetriever._class_session = session
+        return HTTPSPublicKeyRetriever._class_session
 
     def retrieve(self, key_identifier, **requests_kwargs):
         """ returns the public key for given key_identifier. """

atlassian_jwt_auth/tests/test_public_key_provider.py

@@ -1,6 +1,8 @@
+import re
 import unittest
 
 import mock
+import httptest
 import requests
 
 from atlassian_jwt_auth.key import (
@@ -93,6 +95,52 @@ def test_retrieve_fails_with_forbidden_error(self, mock_get_method):
             retriever.retrieve('example/eg')
 
 
+class CachedHTTPPublicKeyRetrieverTest(utils.ES256KeyTestMixin,
+                                       unittest.TestCase):
+
+    class HTTPPublicKeyRetriever(HTTPSPublicKeyRetriever):
+        """A subclass of HTTPSPublicKeyRetriever that allows us to use plain
+        HTTP during testing so we don't have to run an actual SSL server.
+        """
+
+        def __init__(self, base_url):
+            # pretend to the super class that this is an HTTPS url
+            super(CachedHTTPPublicKeyRetrieverTest.HTTPPublicKeyRetriever,
+                  self).__init__(
+                re.sub(r'^http', 'https', base_url, flags=re.IGNORECASE))
+            self.base_url = base_url
+
+    def setUp(self):
+        super(CachedHTTPPublicKeyRetrieverTest, self).setUp()
+        self._private_key_pem = self.get_new_private_key_in_pem_format()
+        self._public_key_pem = utils.get_public_key_pem_for_private_key_pem(
+            self._private_key_pem)
+
+    def test_http_caching(self):
+        """Asserts that our use of requests properly caches keys between
+        invocations across different `HTTPSPublicKeyRetriever` instances.
+        """
+        def wsgi(environ, start_response):
+            print(environ['PATH_INFO'])
+            start_response('200 OK', [
+                ('content-type', 'application/x-pem-file;charset=UTF-8'),
+                ('Cache-Control', 'public,max-age=300,stale-while-revalidate='
+                                  '300,stale-if-error=300'),
+                ('Last-Modified', 'Sun, 18 Jan 1970 18:14:21 GMT')])
+            return [self._public_key_pem]
+
+        with httptest.testserver(wsgi) as server:
+
+            retriever = self.HTTPPublicKeyRetriever(server.url())
+            retriever.retrieve('example/eg')
+
+            retriever = self.HTTPPublicKeyRetriever(server.url())
+            retriever.retrieve('example/eg')
+
+            self.assertEqual(1, len(server.log()),
+                             msg='HTTP caching should suppress second GET')
+
+
 class BaseHTTPSMultiRepositoryPublicKeyRetrieverTest(
         BaseHTTPSPublicKeyRetrieverTest):
     """ tests for the HTTPSMultiRepositoryPublicKeyRetriever class. """

test-requirements.txt

@@ -2,3 +2,4 @@ nose
 mock
 flask<1.1.0
 Django==1.11
+atlassian-httptest==0.8