Skip to content

Commit 3611811

Browse files
author
Tycho Hob
committed
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.
1 parent d8a03a4 commit 3611811

12 files changed

Lines changed: 30 additions & 18 deletions

File tree

Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ PIP_COMPILE = pip-compile $(PIP_COMPILE_OPTS)
3737

3838
compile-requirements: ## compile the requirements/*.txt files with the latest packages satisfying requirements/*.in
3939
pip install -qr requirements/pip-tools.txt
40+
pip install -qr requirements/pip.txt
4041
pip-compile -v ${COMPILE_OPTS} --allow-unsafe --rebuild -o requirements/pip.txt requirements/pip.in
4142
pip-compile -v ${COMPILE_OPTS} -o requirements/pip-tools.txt requirements/pip-tools.in
4243
pip install -qr requirements/pip.txt

openedx_authz/engine/matcher.py

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
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
4+
from edx_django_utils.cache import RequestCache
55

66
from openedx_authz.api.data import ContentLibraryData, ScopeData, UserData
77
from openedx_authz.rest_api.utils import get_user_by_username_or_email
88

9-
RBAC_ADMIN_CACHE_KEY_FMT = "rbac_is_admin_or_superuser_{username}"
10-
RBAC_ADMIN_CACHE_TIMEOUT_SECS = 2
11-
129
User = get_user_model()
1310

1411

@@ -33,26 +30,23 @@ def is_admin_or_superuser_check(request_user: str, request_action: str, request_
3330

3431
scope = ScopeData(namespaced_key=request_scope)
3532
username = UserData(namespaced_key=request_user).external_key
33+
request_cache = RequestCache("rbac_is_admin_or_superuser")
3634

3735
# TODO: This special case for superuser and staff users is currently only for
3836
# content libraries. See: https://github.com/openedx/openedx-authz/issues/87
3937
if not isinstance(scope, ContentLibraryData):
4038
return False
4139

42-
cache_key = RBAC_ADMIN_CACHE_KEY_FMT.format(username=username)
43-
is_allowed = cache.get(cache_key)
44-
45-
if is_allowed is not None:
46-
return is_allowed
40+
cached_response = request_cache.get_cached_response(username)
41+
if cached_response.is_found:
42+
return cached_response.value
4743

4844
try:
4945
user = get_user_by_username_or_email(username)
5046
except User.DoesNotExist:
5147
return False
5248

5349
is_allowed = user.is_staff or user.is_superuser
54-
55-
# TODO: Make this cache timeout configurable
56-
cache.set(cache_key, is_allowed, RBAC_ADMIN_CACHE_TIMEOUT_SECS)
50+
request_cache.set(username, is_allowed)
5751

5852
return is_allowed

openedx_authz/settings/test.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ def plugin_settings(settings): # pylint: disable=unused-argument
4444
"django.contrib.sessions.middleware.SessionMiddleware",
4545
"django.contrib.auth.middleware.AuthenticationMiddleware",
4646
"django.contrib.messages.middleware.MessageMiddleware",
47+
"edx_django_utils.cache.middleware.RequestCacheMiddleware",
4748
]
4849

4950
TEMPLATES = [

requirements/base.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ pycasbin # Authorization library for implementing access cont
99
casbin-django-orm-adapter # Adapter for Django ORM for Casbin
1010
edx-opaque-keys # Opaque keys for resource identification
1111
edx-api-doc-tools # Tools for API documentation
12+
edx-django-utils # Used for RequestCache
1213
edx-drf-extensions # Extensions for Django Rest Framework used by Open edX

requirements/base.txt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ cffi==2.0.0
1919
charset-normalizer==3.4.3
2020
# via requests
2121
click==8.3.0
22-
# via edx-django-utils
22+
# via
23+
# -c requirements/constraints.txt
24+
# edx-django-utils
2325
cryptography==46.0.2
2426
# via pyjwt
2527
django==4.2.24
@@ -57,7 +59,9 @@ drf-yasg==1.21.11
5759
edx-api-doc-tools==2.1.0
5860
# via -r requirements/base.in
5961
edx-django-utils==8.0.1
60-
# via edx-drf-extensions
62+
# via
63+
# -r requirements/base.in
64+
# edx-drf-extensions
6165
edx-drf-extensions==10.6.0
6266
# via -r requirements/base.in
6367
edx-opaque-keys==3.0.0

requirements/constraints.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,6 @@
1010

1111
# Common constraints for edx repos
1212
-c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt
13+
14+
# Different packages want different versions of click, we force the most compatible one here
15+
click==8.3.0

requirements/dev.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ charset-normalizer==3.4.3
4545
# requests
4646
click==8.3.0
4747
# via
48+
# -c requirements/constraints.txt
4849
# -r requirements/pip-tools.txt
4950
# -r requirements/quality.txt
5051
# click-log
@@ -196,7 +197,7 @@ packaging==25.0
196197
# tox
197198
path==16.16.0
198199
# via edx-i18n-tools
199-
pip-tools==7.5.1
200+
pip-tools==7.5.2
200201
# via -r requirements/pip-tools.txt
201202
platformdirs==4.4.0
202203
# via

requirements/doc.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ charset-normalizer==3.4.3
4141
# requests
4242
click==8.3.0
4343
# via
44+
# -c requirements/constraints.txt
4445
# -r requirements/test.txt
4546
# code-annotations
4647
# edx-django-utils

requirements/pip-tools.txt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@
77
build==1.3.0
88
# via pip-tools
99
click==8.3.0
10-
# via pip-tools
10+
# via
11+
# -c requirements/constraints.txt
12+
# pip-tools
1113
packaging==25.0
1214
# via build
13-
pip-tools==7.5.1
15+
pip-tools==7.5.2
1416
# via -r requirements/pip-tools.in
1517
pyproject-hooks==1.2.0
1618
# via

requirements/pip.txt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ wheel==0.45.1
99

1010
# The following packages are considered to be unsafe in a requirements file:
1111
pip==25.2
12-
# via -r requirements/pip.in
12+
# via
13+
# -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt
14+
# -r requirements/pip.in
1315
setuptools==80.9.0
1416
# via -r requirements/pip.in

0 commit comments

Comments
 (0)