From 3015e4ecc88fcb5a5d3ec280a28bae2e5a2ed117 Mon Sep 17 00:00:00 2001 From: Tycho Hob Date: Tue, 25 Nov 2025 08:26:12 -0500 Subject: [PATCH 1/4] 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. --- openedx_authz/engine/matcher.py | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/openedx_authz/engine/matcher.py b/openedx_authz/engine/matcher.py index dea6373d..f5b83712 100644 --- a/openedx_authz/engine/matcher.py +++ b/openedx_authz/engine/matcher.py @@ -1,6 +1,7 @@ """Custom condition checker. Note only used for data_library scope""" from django.contrib.auth import get_user_model +from django.core.cache import cache from openedx_authz.api.data import ContentLibraryData, ScopeData, UserData 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_ ContentLibraryData scopes), False otherwise (including when user doesn't exist or scope type is not supported) """ + + scope = ScopeData(namespaced_key=request_scope) + username = UserData(namespaced_key=request_user).external_key + + # TODO: This special case for superuser and staff users is currently only for + # content libraries. See: https://github.com/openedx/openedx-authz/issues/87 + if not isinstance(scope, ContentLibraryData): + return False + + # TODO: Make this key format a constant + is_allowed = cache.get(f"rbac_is_admin_or_superuser_{username}") + + if is_allowed is not None: + return is_allowed + try: - username = UserData(namespaced_key=request_user).external_key user = get_user_by_username_or_email(username) except User.DoesNotExist: return False - scope = ScopeData(namespaced_key=request_scope) + is_allowed = user.is_staff or user.is_superuser - if isinstance(scope, ContentLibraryData): - return user.is_staff or user.is_superuser + # TODO: Make this cache timeout configurable + cache.set(f"rbac_is_admin_or_superuser_{username}", is_allowed, 2) - return False + return is_allowed From 68e40c3f0b0a9a297d52f661601b5362df109d8c Mon Sep 17 00:00:00 2001 From: Tycho Hob Date: Tue, 25 Nov 2025 13:40:26 -0500 Subject: [PATCH 2/4] refactor: Add RBAC admin cache constants --- openedx_authz/engine/enforcer.py | 2 -- openedx_authz/engine/matcher.py | 9 ++++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/openedx_authz/engine/enforcer.py b/openedx_authz/engine/enforcer.py index a0caad9a..950a1962 100644 --- a/openedx_authz/engine/enforcer.py +++ b/openedx_authz/engine/enforcer.py @@ -151,8 +151,6 @@ def configure_enforcer_auto_save_and_load(cls): if auto_load_policy_interval > 0: cls.configure_enforcer_auto_loading(auto_load_policy_interval) - else: - logger.warning("CASBIN_AUTO_LOAD_POLICY_INTERVAL is not set or zero; auto-load is disabled.") cls.configure_enforcer_auto_save(auto_save_policy) diff --git a/openedx_authz/engine/matcher.py b/openedx_authz/engine/matcher.py index f5b83712..a4540dd0 100644 --- a/openedx_authz/engine/matcher.py +++ b/openedx_authz/engine/matcher.py @@ -6,6 +6,9 @@ from openedx_authz.api.data import ContentLibraryData, ScopeData, UserData from openedx_authz.rest_api.utils import get_user_by_username_or_email +RBAC_ADMIN_CACHE_KEY_FMT = "rbac_is_admin_or_superuser_{username}" +RBAC_ADMIN_CACHE_TIMEOUT_SECS = 2 + User = get_user_model() @@ -36,8 +39,8 @@ def is_admin_or_superuser_check(request_user: str, request_action: str, request_ if not isinstance(scope, ContentLibraryData): return False - # TODO: Make this key format a constant - is_allowed = cache.get(f"rbac_is_admin_or_superuser_{username}") + cache_key = RBAC_ADMIN_CACHE_KEY_FMT.format(username=username) + is_allowed = cache.get(cache_key) if is_allowed is not None: return is_allowed @@ -50,6 +53,6 @@ def is_admin_or_superuser_check(request_user: str, request_action: str, request_ is_allowed = user.is_staff or user.is_superuser # TODO: Make this cache timeout configurable - cache.set(f"rbac_is_admin_or_superuser_{username}", is_allowed, 2) + cache.set(cache_key, is_allowed, RBAC_ADMIN_CACHE_TIMEOUT_SECS) return is_allowed From 630fcd90b466eb7b76418ae4f365e50d6467e298 Mon Sep 17 00:00:00 2001 From: Tycho Hob Date: Tue, 25 Nov 2025 14:20:05 -0500 Subject: [PATCH 3/4] refactor: Use RequestCache for RBAC admin matcher By using the RequestCache instead of the Django cache we are able to have a thread-local memory copy of the user's superuser / staff state that exists only for the length of the request. This will save a large number of round trips to the cache backend. --- Makefile | 1 + openedx_authz/engine/matcher.py | 18 ++++++------------ openedx_authz/settings/test.py | 1 + requirements/base.in | 1 + requirements/base.txt | 8 ++++++-- requirements/constraints.txt | 3 +++ requirements/dev.txt | 3 ++- requirements/doc.txt | 1 + requirements/pip-tools.txt | 6 ++++-- requirements/pip.txt | 4 +++- requirements/quality.txt | 1 + requirements/test.txt | 1 + 12 files changed, 30 insertions(+), 18 deletions(-) diff --git a/Makefile b/Makefile index c48d6e0b..5a276bbb 100644 --- a/Makefile +++ b/Makefile @@ -37,6 +37,7 @@ PIP_COMPILE = pip-compile $(PIP_COMPILE_OPTS) compile-requirements: ## compile the requirements/*.txt files with the latest packages satisfying requirements/*.in pip install -qr requirements/pip-tools.txt + pip install -qr requirements/pip.txt pip-compile -v ${COMPILE_OPTS} --allow-unsafe --rebuild -o requirements/pip.txt requirements/pip.in pip-compile -v ${COMPILE_OPTS} -o requirements/pip-tools.txt requirements/pip-tools.in pip install -qr requirements/pip.txt diff --git a/openedx_authz/engine/matcher.py b/openedx_authz/engine/matcher.py index a4540dd0..d5c3fcf6 100644 --- a/openedx_authz/engine/matcher.py +++ b/openedx_authz/engine/matcher.py @@ -1,14 +1,11 @@ """Custom condition checker. Note only used for data_library scope""" from django.contrib.auth import get_user_model -from django.core.cache import cache +from edx_django_utils.cache import RequestCache from openedx_authz.api.data import ContentLibraryData, ScopeData, UserData from openedx_authz.rest_api.utils import get_user_by_username_or_email -RBAC_ADMIN_CACHE_KEY_FMT = "rbac_is_admin_or_superuser_{username}" -RBAC_ADMIN_CACHE_TIMEOUT_SECS = 2 - User = get_user_model() @@ -33,17 +30,16 @@ def is_admin_or_superuser_check(request_user: str, request_action: str, request_ scope = ScopeData(namespaced_key=request_scope) username = UserData(namespaced_key=request_user).external_key + request_cache = RequestCache("rbac_is_admin_or_superuser") # TODO: This special case for superuser and staff users is currently only for # content libraries. See: https://github.com/openedx/openedx-authz/issues/87 if not isinstance(scope, ContentLibraryData): return False - cache_key = RBAC_ADMIN_CACHE_KEY_FMT.format(username=username) - is_allowed = cache.get(cache_key) - - if is_allowed is not None: - return is_allowed + cached_response = request_cache.get_cached_response(username) + if cached_response.is_found: + return cached_response.value try: user = get_user_by_username_or_email(username) @@ -51,8 +47,6 @@ def is_admin_or_superuser_check(request_user: str, request_action: str, request_ return False is_allowed = user.is_staff or user.is_superuser - - # TODO: Make this cache timeout configurable - cache.set(cache_key, is_allowed, RBAC_ADMIN_CACHE_TIMEOUT_SECS) + request_cache.set(username, is_allowed) return is_allowed diff --git a/openedx_authz/settings/test.py b/openedx_authz/settings/test.py index 8565ceac..5fa633a5 100644 --- a/openedx_authz/settings/test.py +++ b/openedx_authz/settings/test.py @@ -44,6 +44,7 @@ def plugin_settings(settings): # pylint: disable=unused-argument "django.contrib.sessions.middleware.SessionMiddleware", "django.contrib.auth.middleware.AuthenticationMiddleware", "django.contrib.messages.middleware.MessageMiddleware", + "edx_django_utils.cache.middleware.RequestCacheMiddleware", ] TEMPLATES = [ diff --git a/requirements/base.in b/requirements/base.in index 0a5fd473..d10ac489 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -9,4 +9,5 @@ pycasbin # Authorization library for implementing access cont casbin-django-orm-adapter # Adapter for Django ORM for Casbin edx-opaque-keys # Opaque keys for resource identification edx-api-doc-tools # Tools for API documentation +edx-django-utils # Used for RequestCache edx-drf-extensions # Extensions for Django Rest Framework used by Open edX diff --git a/requirements/base.txt b/requirements/base.txt index db6e0c43..a3011807 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -19,7 +19,9 @@ cffi==2.0.0 charset-normalizer==3.4.3 # via requests click==8.3.0 - # via edx-django-utils + # via + # -c requirements/constraints.txt + # edx-django-utils cryptography==46.0.2 # via pyjwt django==4.2.24 @@ -57,7 +59,9 @@ drf-yasg==1.21.11 edx-api-doc-tools==2.1.0 # via -r requirements/base.in edx-django-utils==8.0.1 - # via edx-drf-extensions + # via + # -r requirements/base.in + # edx-drf-extensions edx-drf-extensions==10.6.0 # via -r requirements/base.in edx-opaque-keys==3.0.0 diff --git a/requirements/constraints.txt b/requirements/constraints.txt index d91704bb..ea627da3 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -10,3 +10,6 @@ # Common constraints for edx repos -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt + +# Different packages want different versions of click, we force the most compatible one here +click==8.3.0 diff --git a/requirements/dev.txt b/requirements/dev.txt index 33040c84..ea7acdfc 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -45,6 +45,7 @@ charset-normalizer==3.4.3 # requests click==8.3.0 # via + # -c requirements/constraints.txt # -r requirements/pip-tools.txt # -r requirements/quality.txt # click-log @@ -196,7 +197,7 @@ packaging==25.0 # tox path==16.16.0 # via edx-i18n-tools -pip-tools==7.5.1 +pip-tools==7.5.2 # via -r requirements/pip-tools.txt platformdirs==4.4.0 # via diff --git a/requirements/doc.txt b/requirements/doc.txt index f0e34239..195bf664 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -41,6 +41,7 @@ charset-normalizer==3.4.3 # requests click==8.3.0 # via + # -c requirements/constraints.txt # -r requirements/test.txt # code-annotations # edx-django-utils diff --git a/requirements/pip-tools.txt b/requirements/pip-tools.txt index f87d1e6b..1c069098 100644 --- a/requirements/pip-tools.txt +++ b/requirements/pip-tools.txt @@ -7,10 +7,12 @@ build==1.3.0 # via pip-tools click==8.3.0 - # via pip-tools + # via + # -c requirements/constraints.txt + # pip-tools packaging==25.0 # via build -pip-tools==7.5.1 +pip-tools==7.5.2 # via -r requirements/pip-tools.in pyproject-hooks==1.2.0 # via diff --git a/requirements/pip.txt b/requirements/pip.txt index b6bd229a..8bcb049d 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -9,6 +9,8 @@ wheel==0.45.1 # The following packages are considered to be unsafe in a requirements file: pip==25.2 - # via -r requirements/pip.in + # via + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt + # -r requirements/pip.in setuptools==80.9.0 # via -r requirements/pip.in diff --git a/requirements/quality.txt b/requirements/quality.txt index 9f1325e3..b5099f18 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -31,6 +31,7 @@ charset-normalizer==3.4.3 # requests click==8.3.0 # via + # -c requirements/constraints.txt # -r requirements/test.txt # click-log # code-annotations diff --git a/requirements/test.txt b/requirements/test.txt index d90cfa59..a25f479d 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -27,6 +27,7 @@ charset-normalizer==3.4.3 # requests click==8.3.0 # via + # -c requirements/constraints.txt # -r requirements/base.txt # code-annotations # edx-django-utils From a8350d6343ee3e354839ad20860aa6b347ff5e22 Mon Sep 17 00:00:00 2001 From: Tycho Hob Date: Tue, 25 Nov 2025 14:39:33 -0500 Subject: [PATCH 4/4] chore: Update version and changelog --- CHANGELOG.rst | 8 ++++++++ openedx_authz/__init__.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index feb7d215..6c293f86 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,14 @@ Unreleased * +0.19.2 - 2025-11-25 +******************** + +Performance +=========== + +* Use a RequestCache for is_admin_or_superuser matcher to improve performance. + 0.19.1 - 2025-11-25 ******************** diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index a6e85867..839432d0 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "0.19.1" +__version__ = "0.19.2" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))