Disable memory caching of tokens

For a long time now if you don't configure memcache then auth_token
middleware would cache the tokens in process memory.

This is not the job of auth_token middleware. If you need to cache you
should configure memcache otherwise auth_token will authenticate with
keystone for every token request.

Change-Id: Idf7d864fe8b054738d8a240bc3da377a95eb7e62

keystonemiddleware/auth_token/__init__.py

@@ -802,22 +802,6 @@ class AuthProtocol(BaseAuthProtocol):
         else:
             return [token]
 
-    def _cache_get_hashes(self, token_hashes):
-        """Check if the token is cached already.
-
-        Functions takes a list of hashes that might be in the cache and matches
-        the first one that is present. If nothing is found in the cache it
-        returns None.
-
-        :returns: token data if found else None.
-        """
-
-        for token in token_hashes:
-            cached = self._token_cache.get(token)
-
-            if cached:
-                return cached
-
     def fetch_token(self, token):
         """Retrieve a token from either a PKI bundle or the identity server.
 
@@ -830,7 +814,7 @@ class AuthProtocol(BaseAuthProtocol):
 
         try:
             token_hashes = self._token_hashes(token)
-            cached = self._cache_get_hashes(token_hashes)
+            cached = self._token_cache.get_first(*token_hashes)
 
             if cached:
                 data = cached
@@ -1093,12 +1077,18 @@ class AuthProtocol(BaseAuthProtocol):
             requested_auth_version=auth_version)
 
     def _token_cache_factory(self):
+        memcached_servers = self._conf_get('memcached_servers')
+        env_cache_name = self._conf_get('cache')
+
+        if not (memcached_servers or env_cache_name):
+            return _cache.NoOpCache()
+
         security_strategy = self._conf_get('memcache_security_strategy')
 
         cache_kwargs = dict(
             cache_time=int(self._conf_get('token_cache_time')),
-            env_cache_name=self._conf_get('cache'),
+            env_cache_name=env_cache_name,
-            memcached_servers=self._conf_get('memcached_servers'),
+            memcached_servers=memcached_servers,
             use_advanced_pool=self._conf_get('memcache_use_advanced_pool'),
             dead_retry=self._conf_get('memcache_pool_dead_retry'),
             maxsize=self._conf_get('memcache_pool_maxsize'),

keystonemiddleware/auth_token/_cache.py

@@ -10,6 +10,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import abc
 import contextlib
 import hashlib
 
@@ -20,7 +21,22 @@ from keystonemiddleware.auth_token import _exceptions as exc
 from keystonemiddleware.auth_token import _memcache_crypt as memcache_crypt
 from keystonemiddleware.auth_token import _memcache_pool as memcache_pool
 from keystonemiddleware.i18n import _, _LE
-from keystonemiddleware.openstack.common import memorycache
+
+memcache = None  # module will be loaded on demand to avoid dependency
+
+
+def _create_memcache_client(*args, **kwargs):
+    """Create a new memcache client object.
+
+    This handles the lazy loaded import but also provides a point to mock out
+    in testing.
+    """
+    global memcache
+
+    if not memcache:
+        import memcache
+
+    return memcache.Client(*args, **kwargs)
 
 
 def _hash_key(key):
@@ -63,8 +79,7 @@ class _CachePool(list):
         try:
             c = self.pop()
         except IndexError:
-            # the pool is empty, so we need to create a new client
+            c = _create_memcache_client(self._memcached_servers, debug=0)
-            c = memorycache.get_client(self._memcached_servers)
 
         try:
             yield c
@@ -72,6 +87,57 @@ class _CachePool(list):
             self.append(c)
 
 
+@six.add_metaclass(abc.ABCMeta)
+class _CacheInterface(object):
+
+    def initialize(self, *args, **kwargs):
+        pass
+
+    @abc.abstractmethod
+    def store(self, key, value):
+        pass
+
+    @abc.abstractmethod
+    def store_invalid(self, key):
+        pass
+
+    @abc.abstractmethod
+    def get(self, key):
+        pass
+
+    def get_first(self, *args):
+        """Get the first cached value from many options.
+
+        :returns: token data if found else None.
+        """
+        for a in args:
+            value = self.get(a)
+
+            if value:
+                return value
+
+        return None
+
+
+class NoOpCache(_CacheInterface):
+
+    def store(self, key, value):
+        # Don't store anything
+        return None
+
+    def store_invalid(self, key):
+        # Don't store anything
+        return None
+
+    def get(self, key):
+        # Nothing to fetch from
+        return None
+
+    def get_first(self, *args):
+        # short circuit because calling get() multiple times wont help
+        return None
+
+
 class _MemcacheClientPool(object):
     """An advanced memcached client pool that is eventlet safe."""
     def __init__(self, memcache_servers, **kwargs):
@@ -84,7 +150,7 @@ class _MemcacheClientPool(object):
             yield client
 
 
-class TokenCache(object):
+class TokenCache(_CacheInterface):
     """Encapsulates the auth_token token cache functionality.
 
     auth_token caches tokens that it's seen so that when a token is re-used the
@@ -124,9 +190,14 @@ class TokenCache(object):
             return _MemcacheClientPool(self._memcached_servers,
                                        **self._memcache_pool_options)
 
-        else:
+        elif self._memcached_servers:
             return _CachePool(self._memcached_servers)
 
+        else:
+            raise RuntimeError('Trying to configure a memcache cache without '
+                               'passing any servers. This should have been '
+                               'caught.')
+
     def initialize(self, env):
         if self._initialized:
             return

keystonemiddleware/openstack/common/memorycache.py

@@ -1,97 +0,0 @@
-# Copyright 2010 United States Government as represented by the
-# Administrator of the National Aeronautics and Space Administration.
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-"""Super simple fake memcache client."""
-
-import copy
-
-from oslo_config import cfg
-from oslo_utils import timeutils
-
-memcache_opts = [
-    cfg.ListOpt('memcached_servers',
-                help='Memcached servers or None for in process cache.'),
-]
-
-CONF = cfg.CONF
-CONF.register_opts(memcache_opts)
-
-
-def list_opts():
-    """Entry point for oslo-config-generator."""
-    return [(None, copy.deepcopy(memcache_opts))]
-
-
-def get_client(memcached_servers=None):
-    client_cls = Client
-
-    if not memcached_servers:
-        memcached_servers = CONF.memcached_servers
-    if memcached_servers:
-        import memcache
-        client_cls = memcache.Client
-
-    return client_cls(memcached_servers, debug=0)
-
-
-class Client(object):
-    """Replicates a tiny subset of memcached client interface."""
-
-    def __init__(self, *args, **kwargs):
-        """Ignores the passed in args."""
-        self.cache = {}
-
-    def get(self, key):
-        """Retrieves the value for a key or None.
-
-        This expunges expired keys during each get.
-        """
-
-        now = timeutils.utcnow_ts()
-        for k in list(self.cache):
-            (timeout, _value) = self.cache[k]
-            if timeout and now >= timeout:
-                del self.cache[k]
-
-        return self.cache.get(key, (0, None))[1]
-
-    def set(self, key, value, time=0, min_compress_len=0):
-        """Sets the value for a key."""
-        timeout = 0
-        if time != 0:
-            timeout = timeutils.utcnow_ts() + time
-        self.cache[key] = (timeout, value)
-        return True
-
-    def add(self, key, value, time=0, min_compress_len=0):
-        """Sets the value for a key if it doesn't exist."""
-        if self.get(key) is not None:
-            return False
-        return self.set(key, value, time, min_compress_len)
-
-    def incr(self, key, delta=1):
-        """Increments the value for a key."""
-        value = self.get(key)
-        if value is None:
-            return None
-        new_value = int(value) + delta
-        self.cache[key] = (self.cache[key][0], str(new_value))
-        return new_value
-
-    def delete(self, key, time=0):
-        """Deletes the value associated with a key."""
-        if key in self.cache:
-            del self.cache[key]

keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py

@@ -34,6 +34,7 @@ from oslo_config import cfg
 from oslo_serialization import jsonutils
 from oslo_utils import timeutils
 from oslotest import createfile
+from oslotest import mockpatch
 import six
 import testresources
 import testtools
@@ -45,9 +46,9 @@ from keystonemiddleware import auth_token
 from keystonemiddleware.auth_token import _base
 from keystonemiddleware.auth_token import _exceptions as ksm_exceptions
 from keystonemiddleware.auth_token import _revocations
-from keystonemiddleware.openstack.common import memorycache
 from keystonemiddleware.tests.unit.auth_token import base
 from keystonemiddleware.tests.unit import client_fixtures
+from keystonemiddleware.tests.unit import utils
 
 
 EXPECTED_V2_DEFAULT_ENV_RESPONSE = {
@@ -337,6 +338,11 @@ class BaseAuthTokenMiddlewareTest(base.BaseAuthTokenTestCase):
         else:
             self.assertIsNone(self.requests_mock.last_request)
 
+    def mock_memcache(self):
+        return self.useFixture(mockpatch.Patch(
+            'keystonemiddleware.auth_token._cache._create_memcache_client',
+            return_value=utils.FakeMemcache()))
+
 
 class DiabloAuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest,
                                     testresources.ResourcedTestCase):
@@ -392,14 +398,16 @@ class CachePoolTest(BaseAuthTokenMiddlewareTest):
     def test_not_use_cache_from_env(self):
         # If `swift.cache` is set in the environment but `cache` isn't set
         # initialize the config then the env cache isn't used.
-        self.set_middleware()
+        self.mock_memcache()
+        self.set_middleware(conf={'memcached_servers': ['localhost:4444']})
+
         env = {'swift.cache': 'CACHE_TEST'}
         self.middleware._token_cache.initialize(env)
         with self.middleware._token_cache._cache_pool.reserve() as cache:
             self.assertNotEqual(cache, 'CACHE_TEST')
 
     def test_multiple_context_managers_share_single_client(self):
-        self.set_middleware()
+        self.set_middleware(conf={'memcached_servers': ['localhost:4444']})
         token_cache = self.middleware._token_cache
         env = {}
         token_cache.initialize(env)
@@ -416,7 +424,8 @@ class CachePoolTest(BaseAuthTokenMiddlewareTest):
         self.assertEqual(set(caches), set(token_cache._cache_pool))
 
     def test_nested_context_managers_create_multiple_clients(self):
-        self.set_middleware()
+        self.set_middleware(conf={'memcached_servers': ['localhost:4444']})
+
         env = {}
         self.middleware._token_cache.initialize(env)
         token_cache = self.middleware._token_cache
@@ -461,7 +470,8 @@ class GeneralAuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest,
         self.assertTrue(auth_token._token_is_v3(token_response))
 
     def test_fixed_cache_key_length(self):
-        self.set_middleware()
+        self.set_middleware(conf={'memcached_servers': ['localhost:4444']})
+
         short_string = uuid.uuid4().hex
         long_string = 8 * uuid.uuid4().hex
 
@@ -636,6 +646,9 @@ class CommonAuthTokenMiddlewareTest(object):
 
     def _test_cache_revoked(self, token, revoked_form=None):
         # When the token is cached and revoked, 401 is returned.
+        self.mock_memcache()
+        self.set_middleware(conf={'memcached_servers': ['127.0.0.1:4444']})
+
         self.middleware._check_revocations_for_cached = True
 
         # Token should be cached as ok after this.
@@ -649,6 +662,9 @@ class CommonAuthTokenMiddlewareTest(object):
                              expected_status=401)
 
     def test_cached_revoked_error(self):
+        self.mock_memcache()
+        self.set_middleware(conf={'memcached_servers': ['127.0.0.1:4444']})
+
         # When the token is cached and revocation list retrieval fails,
         # 503 is returned
         token = self.token_dict['uuid_token_default']
@@ -997,6 +1013,8 @@ class CommonAuthTokenMiddlewareTest(object):
         return self.middleware._token_cache.get(token_id)
 
     def test_memcache(self):
+        self.mock_memcache()
+        self.set_middleware(conf={'memcached_servers': ['127.0.0.1:4444']})
         token = self.token_dict['signed_token_scoped']
         self.call_middleware(headers={'X-Auth-Token': token})
         self.assertIsNotNone(self._get_cached_token(token))
@@ -1007,6 +1025,9 @@ class CommonAuthTokenMiddlewareTest(object):
                              expected_status=401)
 
     def test_memcache_set_invalid_uuid(self):
+        self.mock_memcache()
+        self.set_middleware(conf={'memcached_servers': ['127.0.0.1:4444']})
+
         invalid_uri = "%s/v2.0/tokens/invalid-token" % BASE_URI
         self.requests_mock.get(invalid_uri, status_code=404)
 
@@ -1017,8 +1038,11 @@ class CommonAuthTokenMiddlewareTest(object):
                           self._get_cached_token, token)
 
     def test_memcache_set_expired(self, extra_conf={}, extra_environ={}):
+        self.mock_memcache()
+
         token_cache_time = 10
         conf = {
+            'memcached_servers': ['127.0.0.1:4444'],
             'token_cache_time': '%s' % token_cache_time,
         }
         conf.update(extra_conf)
@@ -1041,7 +1065,7 @@ class CommonAuthTokenMiddlewareTest(object):
 
     def test_swift_memcache_set_expired(self):
         extra_conf = {'cache': 'swift.cache'}
-        extra_environ = {'swift.cache': memorycache.Client()}
+        extra_environ = {'swift.cache': utils.FakeMemcache()}
         self.test_memcache_set_expired(extra_conf, extra_environ)
 
     def test_http_error_not_cached_token(self):
@@ -1243,8 +1267,8 @@ class CommonAuthTokenMiddlewareTest(object):
         # When the token is cached it isn't cached again when it's verified.
 
         # The token cache has to be initialized with our cache instance.
-        self.middleware._token_cache._env_cache_name = 'cache'
+        self.set_middleware(conf={'cache': 'cache'})
-        cache = memorycache.Client()
+        cache = utils.FakeMemcache()
         self.middleware._token_cache.initialize(env={'cache': cache})
 
         # Mock cache.set since then the test can verify call_count.
@@ -1825,9 +1849,11 @@ class v3AuthTokenMiddlewareTest(BaseAuthTokenMiddlewareTest,
     def test_expire_stored_in_cache(self):
         # tests the upgrade path from storing a tuple vs just the data in the
         # cache. Can be removed in the future.
+        self.mock_memcache()
+
         token = 'mytoken'
         data = 'this_data'
-        self.set_middleware()
+        self.set_middleware(conf={'memcached_servers': ['localhost:4444']})
         self.middleware._token_cache.initialize({})
         now = datetime.datetime.utcnow()
         delta = datetime.timedelta(hours=1)

keystonemiddleware/tests/unit/utils.py

@@ -17,6 +17,7 @@ import warnings
 
 import fixtures
 import mock
+from oslo_utils import timeutils
 import oslotest.base as oslotest
 import requests
 import uuid
@@ -150,3 +151,53 @@ class NoModuleFinder(object):
     def find_module(self, fullname, path):
         if fullname == self.module or fullname.startswith(self.module + '.'):
             raise ImportError
+
+
+class FakeMemcache(object):
+    """Replicates a tiny subset of memcached client interface."""
+
+    def __init__(self, *args, **kwargs):
+        """Ignores the passed in args."""
+        self.cache = {}
+
+    def get(self, key):
+        """Retrieves the value for a key or None.
+
+        This expunges expired keys during each get.
+        """
+
+        now = timeutils.utcnow_ts()
+        for k in list(self.cache):
+            (timeout, _value) = self.cache[k]
+            if timeout and now >= timeout:
+                del self.cache[k]
+
+        return self.cache.get(key, (0, None))[1]
+
+    def set(self, key, value, time=0, min_compress_len=0):
+        """Sets the value for a key."""
+        timeout = 0
+        if time != 0:
+            timeout = timeutils.utcnow_ts() + time
+        self.cache[key] = (timeout, value)
+        return True
+
+    def add(self, key, value, time=0, min_compress_len=0):
+        """Sets the value for a key if it doesn't exist."""
+        if self.get(key) is not None:
+            return False
+        return self.set(key, value, time, min_compress_len)
+
+    def incr(self, key, delta=1):
+        """Increments the value for a key."""
+        value = self.get(key)
+        if value is None:
+            return None
+        new_value = int(value) + delta
+        self.cache[key] = (self.cache[key][0], str(new_value))
+        return new_value
+
+    def delete(self, key, time=0):
+        """Deletes the value associated with a key."""
+        if key in self.cache:
+            del self.cache[key]

openstack-common.conf

@@ -1,7 +1,6 @@
 [DEFAULT]
 
 # The list of modules to copy from oslo-incubator
-module=memorycache
 
 # The base module to hold the copy of openstack.common
 base=keystonemiddleware