Don't cache signed tokens
Currently the cache is always checked and a result is always cached even if it is for a PKI token. In the case of PKI token though we don't have to do an online lookup, we just decoded the data we've been given. There is a CPU vs IO bound tradeoff as to whether you take decrypt the token or wait for memcache to return however I do not see any point in storing the result of a signing operation. This will also make it much easier to support similar signing validations on the keystone server side because we are not bound by the same cache constraints. Change-Id: Ibb45b14954ef8a26dcc0e3b5dfa7c047dd8edfe1
This commit is contained in:
parent
f27d7f776e
commit
5ba3d06b20
@ -809,26 +809,28 @@ class AuthProtocol(BaseAuthProtocol):
|
||||
|
||||
:raises exc.InvalidToken: if token is rejected
|
||||
"""
|
||||
data = None
|
||||
token_hashes = None
|
||||
|
||||
try:
|
||||
token_hashes = self._token_hashes(token)
|
||||
offline_data = self._validate_offline(token, token_hashes)
|
||||
|
||||
if offline_data:
|
||||
# NOTE(jamielennox): If we've validated a PKI token we don't
|
||||
# need to cache it, and revocation check was already performed.
|
||||
return offline_data
|
||||
|
||||
cached = self._token_cache.get_first(*token_hashes)
|
||||
|
||||
if cached:
|
||||
data = cached
|
||||
|
||||
if self._check_revocations_for_cached:
|
||||
# A token might have been revoked, regardless of initial
|
||||
# mechanism used to validate it, and needs to be checked.
|
||||
self._revocations.check(token_hashes)
|
||||
else:
|
||||
data = self._validate_offline(token, token_hashes)
|
||||
if not data:
|
||||
data = self._identity_server.verify_token(token)
|
||||
|
||||
self._token_cache.store(token_hashes[0], data)
|
||||
return cached
|
||||
|
||||
data = self._identity_server.verify_token(token)
|
||||
self._token_cache.store(token_hashes[0], data)
|
||||
return data
|
||||
|
||||
except (ksa_exceptions.ConnectFailure,
|
||||
ksa_exceptions.RequestTimeout,
|
||||
@ -846,8 +848,6 @@ class AuthProtocol(BaseAuthProtocol):
|
||||
self.log.critical(_LC('Unable to validate token'), exc_info=True)
|
||||
raise webob.exc.HTTPInternalServerError()
|
||||
|
||||
return data
|
||||
|
||||
def _validate_offline(self, token, token_hashes):
|
||||
try:
|
||||
if cms.is_pkiz(token):
|
||||
|
@ -1015,7 +1015,7 @@ class CommonAuthTokenMiddlewareTest(object):
|
||||
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']
|
||||
token = self.token_dict['uuid_token_default']
|
||||
self.call_middleware(headers={'X-Auth-Token': token})
|
||||
self.assertIsNotNone(self._get_cached_token(token))
|
||||
|
||||
@ -1048,7 +1048,7 @@ class CommonAuthTokenMiddlewareTest(object):
|
||||
conf.update(extra_conf)
|
||||
self.set_middleware(conf=conf)
|
||||
|
||||
token = self.token_dict['signed_token_scoped']
|
||||
token = self.token_dict['uuid_token_default']
|
||||
self.call_middleware(headers={'X-Auth-Token': token})
|
||||
|
||||
req = webob.Request.blank('/')
|
||||
@ -1275,7 +1275,7 @@ class CommonAuthTokenMiddlewareTest(object):
|
||||
orig_cache_set = cache.set
|
||||
cache.set = mock.Mock(side_effect=orig_cache_set)
|
||||
|
||||
token = self.token_dict['signed_token_scoped']
|
||||
token = self.token_dict['uuid_token_default']
|
||||
|
||||
self.call_middleware(headers={'X-Auth-Token': token})
|
||||
|
||||
@ -1286,6 +1286,21 @@ class CommonAuthTokenMiddlewareTest(object):
|
||||
# Assert that the token wasn't cached again.
|
||||
self.assertThat(1, matchers.Equals(cache.set.call_count))
|
||||
|
||||
def test_dont_cache_pki_tokens(self):
|
||||
cache = mock.Mock()
|
||||
cache.get.return_value = '{}'
|
||||
|
||||
self.middleware._token_cache._env_cache_name = 'cache'
|
||||
self.middleware._token_cache.initialize(env={'cache': cache})
|
||||
|
||||
token = self.token_dict['signed_token_scoped']
|
||||
|
||||
resp = self.call_middleware(headers={'X-Auth-Token': token})
|
||||
self.assertEqual(200, resp.status_int)
|
||||
|
||||
cache.get.assert_not_called()
|
||||
cache.set.assert_not_called()
|
||||
|
||||
def test_auth_plugin(self):
|
||||
|
||||
for service_url in (self.examples.UNVERSIONED_SERVICE_URL,
|
||||
|
Loading…
x
Reference in New Issue
Block a user