From 86cf26310a9a049ab6bbd59f3e8a29ba1fe86b17 Mon Sep 17 00:00:00 2001
From: Lance Bragstad <lbragstad@gmail.com>
Date: Wed, 6 Mar 2019 22:17:15 +0000
Subject: [PATCH] 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)
---
 .../middleware/auth_context.py                | 16 +++++++------
 keystone/tests/unit/test_middleware.py        | 23 +++++++++++++++++++
 .../notes/bug-1819036-e2d24655c70d0aad.yaml   |  9 ++++++++
 3 files changed, 41 insertions(+), 7 deletions(-)
 create mode 100644 releasenotes/notes/bug-1819036-e2d24655c70d0aad.yaml

diff --git a/keystone/server/flask/request_processing/middleware/auth_context.py b/keystone/server/flask/request_processing/middleware/auth_context.py
index 7961416d3e..56f1df397c 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 8b4aa36450..047d7d5e1f 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 0000000000..adf7fb692b
--- /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.