Skip to content

Commit 2c7d843

Browse files
CopilotMaferMazu
andauthored
perf: add per-request RequestCache to is_user_allowed
Agent-Logs-Url: https://github.com/openedx/openedx-authz/sessions/c95aaea6-7134-4d75-bb96-f9ab7e1eda4d Co-authored-by: MaferMazu <[email protected]>
1 parent f4962be commit 2c7d843

4 files changed

Lines changed: 93 additions & 2 deletions

File tree

CHANGELOG.rst

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,18 @@ Change Log
1414
Unreleased
1515
**********
1616

17+
1.15.0 - 2026-04-24
18+
*******************
19+
20+
Performance
21+
===========
22+
23+
* Add a per-request ``RequestCache`` to ``is_user_allowed`` to prevent redundant Casbin
24+
enforcement calls when the same ``(user, action, scope)`` triple is checked multiple times
25+
within a single HTTP request (e.g., once per object-tag during serialization).
26+
The cache is automatically cleared whenever a role assignment changes via the user API
27+
so that permission checks within the same request always reflect the current state.
28+
1729
1.14.0 - 2026-04-22
1830
*******************
1931

openedx_authz/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44

55
import os
66

7-
__version__ = "1.14.0"
7+
__version__ = "1.15.0"
88

99
ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))

openedx_authz/api/users.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
from django.contrib.auth import get_user_model
1313
from django.db.models import Q
14+
from edx_django_utils.cache import RequestCache
1415

1516
from openedx_authz.api.data import (
1617
ActionData,
@@ -43,6 +44,10 @@
4344

4445
User = get_user_model()
4546

47+
# Cache namespace used by is_user_allowed. Cleared by role mutation functions so
48+
# that permission checks within the same request reflect the latest assignments.
49+
_IS_USER_ALLOWED_CACHE_NS = "rbac_is_user_allowed"
50+
4651

4752
__all__ = [
4853
"assign_role_to_user_in_scope",
@@ -76,6 +81,7 @@ def assign_role_to_user_in_scope(user_external_key: str, role_external_key: str,
7681
Returns:
7782
bool: True if the role was assigned successfully, False otherwise.
7883
"""
84+
RequestCache(_IS_USER_ALLOWED_CACHE_NS).clear()
7985
return assign_role_to_subject_in_scope(
8086
UserData(external_key=user_external_key),
8187
RoleData(external_key=role_external_key),
@@ -91,6 +97,7 @@ def batch_assign_role_to_users_in_scope(users: list[str], role_external_key: str
9197
role_external_key (str): Name of the role to assign.
9298
scope (str): Scope in which to assign the role.
9399
"""
100+
RequestCache(_IS_USER_ALLOWED_CACHE_NS).clear()
94101
namespaced_users = [UserData(external_key=username) for username in users]
95102
batch_assign_role_to_subjects_in_scope(
96103
namespaced_users,
@@ -110,6 +117,7 @@ def unassign_role_from_user(user_external_key: str, role_external_key: str, scop
110117
Returns:
111118
bool: True if the role was unassigned successfully, False otherwise.
112119
"""
120+
RequestCache(_IS_USER_ALLOWED_CACHE_NS).clear()
113121
return unassign_role_from_subject_in_scope(
114122
UserData(external_key=user_external_key),
115123
RoleData(external_key=role_external_key),
@@ -125,6 +133,7 @@ def batch_unassign_role_from_users(users: list[str], role_external_key: str, sco
125133
role_external_key (str): Name of the role to unassign.
126134
scope (str): Scope in which to unassign the role.
127135
"""
136+
RequestCache(_IS_USER_ALLOWED_CACHE_NS).clear()
128137
namespaced_users = [UserData(external_key=user) for user in users]
129138
batch_unassign_role_from_subjects_in_scope(
130139
namespaced_users,
@@ -345,6 +354,11 @@ def is_user_allowed(
345354
) -> bool:
346355
"""Check if a user has a specific permission in a given scope.
347356
357+
Results are cached per-request (keyed by user, action, and scope) to avoid
358+
repeated enforcement calls for the same arguments within a single request,
359+
e.g. when permission checks are performed once per object-tag during
360+
serialization.
361+
348362
Args:
349363
user_external_key (str): ID of the user (e.g., 'john_doe').
350364
action_external_key (str): The action to check (e.g., 'view_course').
@@ -353,11 +367,20 @@ def is_user_allowed(
353367
Returns:
354368
bool: True if the user has the specified permission in the scope, False otherwise.
355369
"""
356-
return is_subject_allowed(
370+
request_cache = RequestCache(_IS_USER_ALLOWED_CACHE_NS)
371+
cache_key = f"{user_external_key}:{action_external_key}:{scope_external_key}"
372+
373+
cached_response = request_cache.get_cached_response(cache_key)
374+
if cached_response.is_found:
375+
return cached_response.value
376+
377+
result = is_subject_allowed(
357378
UserData(external_key=user_external_key),
358379
ActionData(external_key=action_external_key),
359380
ScopeData(external_key=scope_external_key),
360381
)
382+
request_cache.set(cache_key, result)
383+
return result
361384

362385

363386
def get_users_for_role_in_scope(role_external_key: str, scope_external_key: str) -> list[UserData]:
@@ -405,6 +428,7 @@ def unassign_all_roles_from_user(user_external_key: str) -> bool:
405428
Returns:
406429
bool: True if any roles were removed, False otherwise.
407430
"""
431+
RequestCache(_IS_USER_ALLOWED_CACHE_NS).clear()
408432
return unassign_subject_from_all_roles(UserData(external_key=user_external_key))
409433

410434

openedx_authz/tests/api/test_users.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from ddt import data, ddt, unpack
66
from django.contrib.auth import get_user_model
7+
from edx_django_utils.cache import RequestCache
78

89
from openedx_authz.api.data import ContentLibraryData, RoleAssignmentData, RoleData, UserData
910
from openedx_authz.api.users import (
@@ -521,6 +522,60 @@ def test_is_user_allowed(self, username, action, scope_name, expected_result):
521522
self.assertEqual(result, expected_result)
522523

523524

525+
class TestIsUserAllowedRequestCache(UserAssignmentsSetupMixin):
526+
"""Test that is_user_allowed uses a per-request cache to avoid redundant enforcement calls."""
527+
528+
def setUp(self):
529+
super().setUp()
530+
# Clear the request cache before each test so results don't bleed across tests.
531+
RequestCache.clear_all_namespaces()
532+
533+
def test_cache_hit_on_repeated_call(self):
534+
"""Repeated calls with identical arguments should only invoke the enforcer once."""
535+
username = "alice"
536+
action = permissions.DELETE_LIBRARY.identifier
537+
scope = "lib:Org1:math_101"
538+
539+
with patch("openedx_authz.api.users.is_subject_allowed") as mock_enforce:
540+
mock_enforce.return_value = True
541+
first = is_user_allowed(user_external_key=username, action_external_key=action, scope_external_key=scope)
542+
second = is_user_allowed(user_external_key=username, action_external_key=action, scope_external_key=scope)
543+
544+
self.assertTrue(first)
545+
self.assertEqual(first, second)
546+
# The underlying enforcement should have been invoked only once; the second call is served from cache.
547+
self.assertEqual(mock_enforce.call_count, 1)
548+
549+
def test_cache_miss_on_different_scope(self):
550+
"""Different scope arguments must produce independent cache entries."""
551+
username = "alice"
552+
action = permissions.DELETE_LIBRARY.identifier
553+
554+
with patch("openedx_authz.api.users.is_subject_allowed") as mock_enforce:
555+
mock_enforce.return_value = True
556+
is_user_allowed(user_external_key=username, action_external_key=action, scope_external_key="lib:Org1:math_101")
557+
is_user_allowed(user_external_key=username, action_external_key=action, scope_external_key="lib:Org1:math_advanced")
558+
559+
# Each distinct scope key is a separate cache entry → two enforcer calls.
560+
self.assertEqual(mock_enforce.call_count, 2)
561+
562+
def test_cache_cleared_after_role_mutation(self):
563+
"""The cache must be invalidated when a role assignment changes within the same request."""
564+
username = "alice"
565+
action = permissions.DELETE_LIBRARY.identifier
566+
scope = "lib:Org1:math_101"
567+
568+
# alice has the permission before unassignment
569+
before = is_user_allowed(user_external_key=username, action_external_key=action, scope_external_key=scope)
570+
self.assertTrue(before)
571+
572+
# Unassigning roles should clear the cache so the next call goes to the enforcer.
573+
unassign_all_roles_from_user(user_external_key=username)
574+
575+
after = is_user_allowed(user_external_key=username, action_external_key=action, scope_external_key=scope)
576+
self.assertFalse(after)
577+
578+
524579
@ddt
525580
class TestValidateUsersAPI(UserAssignmentsSetupMixin):
526581
"""Test suite for validate_users API function - focused on business logic."""

0 commit comments

Comments
 (0)