Skip to content

Commit 934807f

Browse files
author
Tycho Hob
committed
refactor: cache the results of is_admin_or_superuser_check
is_admin_or_superuser_check is being called once per policy when checking enforcement, creating a potential performance issue with numerous calls to the database. This adds a brief cache to offload some of the burden, but we will need a better fix long term.
1 parent 125894f commit 934807f

1 file changed

Lines changed: 20 additions & 5 deletions

File tree

openedx_authz/engine/matcher.py

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Custom condition checker. Note only used for data_library scope"""
22

33
from django.contrib.auth import get_user_model
4+
from django.core.cache import cache
45

56
from openedx_authz.api.data import ContentLibraryData, ScopeData, UserData
67
from openedx_authz.rest_api.utils import get_user_by_username_or_email
@@ -26,15 +27,29 @@ def is_admin_or_superuser_check(request_user: str, request_action: str, request_
2627
ContentLibraryData scopes), False otherwise (including when user
2728
doesn't exist or scope type is not supported)
2829
"""
30+
31+
scope = ScopeData(namespaced_key=request_scope)
32+
username = UserData(namespaced_key=request_user).external_key
33+
34+
# TODO: This special case for superuser and staff users is currently only for
35+
# content libraries. See: https://github.com/openedx/openedx-authz/issues/87
36+
if not isinstance(scope, ContentLibraryData):
37+
return False
38+
39+
# TODO: Make this key format a constant
40+
is_allowed = cache.get(f"rbac_is_admin_or_superuser_{username}")
41+
42+
if is_allowed is not None:
43+
return is_allowed
44+
2945
try:
30-
username = UserData(namespaced_key=request_user).external_key
3146
user = get_user_by_username_or_email(username)
3247
except User.DoesNotExist:
3348
return False
3449

35-
scope = ScopeData(namespaced_key=request_scope)
50+
is_allowed = user.is_staff or user.is_superuser
3651

37-
if isinstance(scope, ContentLibraryData):
38-
return user.is_staff or user.is_superuser
52+
# TODO: Make this cache timeout configurable
53+
cache.set(f"rbac_is_admin_or_superuser_{username}", is_allowed, 2)
3954

40-
return False
55+
return is_allowed

0 commit comments

Comments
 (0)