Add request-level caching for privileged queries.

dashboard/dashboard/datastore_hooks.py

Index: dashboard/dashboard/datastore_hooks.py
diff --git a/dashboard/dashboard/datastore_hooks.py b/dashboard/dashboard/datastore_hooks.py
index 9c612a630c207bd428efbe4cf5e47c8c6c171bb7..a55a3f458d00082ad5661641e9e921bbc1512e5e 100644
--- a/dashboard/dashboard/datastore_hooks.py
+++ b/dashboard/dashboard/datastore_hooks.py
@@ -73,13 +73,8 @@ def CancelSinglePrivilegedRequest():
   request.registry['single_privileged'] = False
 
 
-def _IsServicingPrivilegedRequest():
+def _IsServicingPrivilegedRequest(request):
   """Checks whether the request is considered privileged."""
-  try:
-    request = webapp2.get_request()
-  except AssertionError:
-    # This happens in unit tests, when code gets called outside of a request.
-    return False
   path = getattr(request, 'path', '')
   if path.startswith('/mapreduce'):
     return True
@@ -89,9 +84,6 @@ def _IsServicingPrivilegedRequest():
     return True
   if request.registry.get('privileged', False):
     return True
-  if request.registry.get('single_privileged', False):
-    request.registry['single_privileged'] = False
-    return True
   whitelist = utils.GetIpWhitelist()
   if whitelist and hasattr(request, 'remote_addr'):
     return request.remote_addr in whitelist
@@ -108,13 +100,41 @@ def IsUnalteredQueryPermitted():
   Returns:
     True for users with google.com emails and privileged requests.
   """
-  if utils.IsInternalUser():
+  try:
+    request = webapp2.get_request()
+  except AssertionError:
+    # This happens in unit tests, when code gets called outside of a request.
     return True

[sullivan, 2016/09/19 21:40:27]

Switched this from False->True because it turns out a lot more unit tests hit
this code path when I move it up a level, and the assumption has been that it's
True in those tests.

-  if users.is_current_user_admin():
+
+  cached_privileged = request.registry.get('privileged_cached', None)
+  if cached_privileged == True or cached_privileged == False:
+    return cached_privileged
+
+  privileged = False
+  new_cached_privileged = None
+  if utils.IsInternalUser():
+    privileged = True
+    new_cached_privileged = True
+  elif users.is_current_user_admin():
     # It's possible to be an admin with a non-internal account; For example,
     # the default login for dev appserver instances is test@example.com.
-    return True
-  return _IsServicingPrivilegedRequest()
+    privileged = True
+    new_cached_privileged = True
+  elif request.registry.get('single_privileged', False):
+    # This request was set to privileged for just one single query. Allow
+    # that query and then reset.
+    privileged = True
+    request.registry['single_privileged'] = False
+    # DO NOT CACHE the result of a SINGLE privilige elevation.

[benjhayden, 2016/09/19 22:11:10]

privil_e_ge

[sullivan, 2016/09/20 02:19:32]

Done.

+    new_cached_privileged = None
+  else:
+    privileged = _IsServicingPrivilegedRequest(request)
+    new_cached_privileged = privileged
+
+  if new_cached_privileged != None:
+    request.registry['privileged_cached'] = new_cached_privileged
+
+  return privileged
 
 
 def GetNamespace():