diff options
author | Lance Bragstad <lbragstad@gmail.com> | 2019-03-06 22:17:15 +0000 |
---|---|---|
committer | Lance Bragstad <lbragstad@gmail.com> | 2019-03-25 17:41:34 +0000 |
commit | 86cf26310a9a049ab6bbd59f3e8a29ba1fe86b17 (patch) | |
tree | cfc4640cb1013c8e27e2df32dd6dae3d675446ac | |
parent | 1f1522703cfa25a533b7462aa402ac30128473e3 (diff) | |
download | keystone-86cf26310a9a049ab6bbd59f3e8a29ba1fe86b17.tar.gz |
Only validate tokens once per request
Keystone actually validates each token twice for every API request.
Regardless of caching being configured, we have an opportunity to try
and spend less time doing something we've already done.
The first the token is validated is actually done through a
keystonemiddleware hook. The second time is to populate a context
object that we can use for things like policy decisions.
Closes-Bug: 1819036
Change-Id: Ifd7f6f0a1dcd33ad17646cae383132cfc2462f03
(cherry picked from commit 112fa29a7472d8d51f2bc920fc3a13fea8d6e8b8)
-rw-r--r-- | keystone/server/flask/request_processing/middleware/auth_context.py | 16 | ||||
-rw-r--r-- | keystone/tests/unit/test_middleware.py | 23 | ||||
-rw-r--r-- | releasenotes/notes/bug-1819036-e2d24655c70d0aad.yaml | 9 |
3 files changed, 41 insertions, 7 deletions
diff --git a/keystone/server/flask/request_processing/middleware/auth_context.py b/keystone/server/flask/request_processing/middleware/auth_context.py index 7961416d3..56f1df397 100644 --- a/keystone/server/flask/request_processing/middleware/auth_context.py +++ b/keystone/server/flask/request_processing/middleware/auth_context.py @@ -237,11 +237,12 @@ class AuthContextMiddleware(provider_api.ProviderAPIMixin, def __init__(self, app): super(AuthContextMiddleware, self).__init__(app, log=LOG) + self.token = None def fetch_token(self, token, **kwargs): try: - token_model = self.token_provider_api.validate_token(token) - return render_token.render_token_response_from_model(token_model) + self.token = self.token_provider_api.validate_token(token) + return render_token.render_token_response_from_model(self.token) except exception.TokenNotFound: raise auth_token.InvalidToken(_('Could not find token')) @@ -416,10 +417,11 @@ class AuthContextMiddleware(provider_api.ProviderAPIMixin, elif request.token_auth.has_user_token: # Keystone enforces policy on some values that other services # do not, and should not, use. This adds them in to the context. - token = PROVIDERS.token_provider_api.validate_token( - request.user_token - ) - self._keystone_specific_values(token, request_context) + if not self.token: + self.token = PROVIDERS.token_provider_api.validate_token( + request.user_token + ) + self._keystone_specific_values(self.token, request_context) request_context.auth_token = request.user_token auth_context = request_context.to_policy_values() additional = { @@ -429,7 +431,7 @@ class AuthContextMiddleware(provider_api.ProviderAPIMixin, 'domain_id': request_context._domain_id, 'domain_name': request_context.domain_name, 'group_ids': request_context.group_ids, - 'token': token + 'token': self.token } auth_context.update(additional) diff --git a/keystone/tests/unit/test_middleware.py b/keystone/tests/unit/test_middleware.py index 8b4aa3645..047d7d5e1 100644 --- a/keystone/tests/unit/test_middleware.py +++ b/keystone/tests/unit/test_middleware.py @@ -17,9 +17,11 @@ import hashlib import uuid import fixtures +import mock from six.moves import http_client import webtest +from keystone.auth import core as auth_core from keystone.common import authorization from keystone.common import context as keystone_context from keystone.common import provider_api @@ -730,3 +732,24 @@ class AuthContextMiddlewareTest(test_backend_sql.SqlTests, headers = {authorization.AUTH_TOKEN_HEADER: 'NOT-ADMIN'} self._do_middleware_request(headers=headers) self.assertIn('Invalid user token', log_fix.output) + + def test_token_is_cached(self): + # Make sure we only call PROVIDERS.token_provider_api.validate_token() + # once while in middleware so that we're mindful of performance + context = auth_core.AuthContext( + user_id=self.user['id'], methods=['password'] + ) + token = PROVIDERS.token_provider_api.issue_token( + context['user_id'], context['methods'], project_id=self.project_id, + auth_context=context + ) + headers = { + authorization.AUTH_TOKEN_HEADER: token.id.encode('utf-8') + } + with mock.patch.object(PROVIDERS.token_provider_api, + 'validate_token', + return_value=token) as token_mock: + self._do_middleware_request( + path='/v3/projects', method='get', headers=headers + ) + token_mock.assert_called_once() diff --git a/releasenotes/notes/bug-1819036-e2d24655c70d0aad.yaml b/releasenotes/notes/bug-1819036-e2d24655c70d0aad.yaml new file mode 100644 index 000000000..adf7fb692 --- /dev/null +++ b/releasenotes/notes/bug-1819036-e2d24655c70d0aad.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + [`bug 1819036 <https://bugs.launchpad.net/keystone/+bug/1819036>`_] + Middleware that processes requests in front of keystone now caches tokens + per request, eliminating unnecessary round trips to validate tokens on + every request. This change doesn't require the usage of any configuration + options to take effect. The fix for this bug improved performance ~20% during + testing and impacts most of keystone's API. |