Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3561)

Unified Diff: dashboard/dashboard/datastore_hooks.py

Issue 2350113002: Add request-level caching for privileged queries. (Closed)
Patch Set: Addressed review comments Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | dashboard/dashboard/datastore_hooks_test.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: dashboard/dashboard/datastore_hooks.py
diff --git a/dashboard/dashboard/datastore_hooks.py b/dashboard/dashboard/datastore_hooks.py
index 9c612a630c207bd428efbe4cf5e47c8c6c171bb7..b7a28f9b567f09fb2a9b73645ea87d788d13a5b5 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
- 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 privilege elevation.
+ 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():
« no previous file with comments | « no previous file | dashboard/dashboard/datastore_hooks_test.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698