Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,18 @@ Change Log
Unreleased
**********

1.15.0 - 2026-04-24
*******************

Performance
===========

* Add a per-request ``RequestCache`` to ``is_user_allowed`` to prevent redundant Casbin
enforcement calls when the same ``(user, action, scope)`` triple is checked multiple times
within a single HTTP request (e.g., once per object-tag during serialization).
The cache is automatically cleared whenever a role assignment changes via the user API
so that permission checks within the same request always reflect the current state.

1.14.0 - 2026-04-22
*******************

Expand Down
2 changes: 1 addition & 1 deletion openedx_authz/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

import os

__version__ = "1.14.0"
__version__ = "1.15.0"

ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))
26 changes: 25 additions & 1 deletion openedx_authz/api/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

from django.contrib.auth import get_user_model
from django.db.models import Q
from edx_django_utils.cache import RequestCache

from openedx_authz.api.data import (
ActionData,
Expand Down Expand Up @@ -43,6 +44,10 @@

User = get_user_model()

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


__all__ = [
"assign_role_to_user_in_scope",
Expand Down Expand Up @@ -76,6 +81,7 @@ def assign_role_to_user_in_scope(user_external_key: str, role_external_key: str,
Returns:
bool: True if the role was assigned successfully, False otherwise.
"""
RequestCache(_IS_USER_ALLOWED_CACHE_NS).clear()
return assign_role_to_subject_in_scope(
UserData(external_key=user_external_key),
RoleData(external_key=role_external_key),
Expand All @@ -91,6 +97,7 @@ def batch_assign_role_to_users_in_scope(users: list[str], role_external_key: str
role_external_key (str): Name of the role to assign.
scope (str): Scope in which to assign the role.
"""
RequestCache(_IS_USER_ALLOWED_CACHE_NS).clear()
namespaced_users = [UserData(external_key=username) for username in users]
batch_assign_role_to_subjects_in_scope(
namespaced_users,
Expand All @@ -110,6 +117,7 @@ def unassign_role_from_user(user_external_key: str, role_external_key: str, scop
Returns:
bool: True if the role was unassigned successfully, False otherwise.
"""
RequestCache(_IS_USER_ALLOWED_CACHE_NS).clear()
return unassign_role_from_subject_in_scope(
UserData(external_key=user_external_key),
RoleData(external_key=role_external_key),
Expand All @@ -125,6 +133,7 @@ def batch_unassign_role_from_users(users: list[str], role_external_key: str, sco
role_external_key (str): Name of the role to unassign.
scope (str): Scope in which to unassign the role.
"""
RequestCache(_IS_USER_ALLOWED_CACHE_NS).clear()
namespaced_users = [UserData(external_key=user) for user in users]
batch_unassign_role_from_subjects_in_scope(
namespaced_users,
Expand Down Expand Up @@ -345,6 +354,11 @@ def is_user_allowed(
) -> bool:
"""Check if a user has a specific permission in a given scope.

Results are cached per-request (keyed by user, action, and scope) to avoid
repeated enforcement calls for the same arguments within a single request,
e.g. when permission checks are performed once per object-tag during
serialization.

Args:
user_external_key (str): ID of the user (e.g., 'john_doe').
action_external_key (str): The action to check (e.g., 'view_course').
Expand All @@ -353,11 +367,20 @@ def is_user_allowed(
Returns:
bool: True if the user has the specified permission in the scope, False otherwise.
"""
return is_subject_allowed(
request_cache = RequestCache(_IS_USER_ALLOWED_CACHE_NS)
cache_key = f"{user_external_key}:{action_external_key}:{scope_external_key}"

cached_response = request_cache.get_cached_response(cache_key)
if cached_response.is_found:
return cached_response.value

result = is_subject_allowed(
UserData(external_key=user_external_key),
ActionData(external_key=action_external_key),
ScopeData(external_key=scope_external_key),
)
request_cache.set(cache_key, result)
return result


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


Expand Down
59 changes: 59 additions & 0 deletions openedx_authz/tests/api/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from ddt import data, ddt, unpack
from django.contrib.auth import get_user_model
from edx_django_utils.cache import RequestCache

from openedx_authz.api.data import ContentLibraryData, RoleAssignmentData, RoleData, UserData
from openedx_authz.api.users import (
Expand Down Expand Up @@ -521,6 +522,64 @@ def test_is_user_allowed(self, username, action, scope_name, expected_result):
self.assertEqual(result, expected_result)


class TestIsUserAllowedRequestCache(UserAssignmentsSetupMixin):
"""Test that is_user_allowed uses a per-request cache to avoid redundant enforcement calls."""

def setUp(self):
super().setUp()
# Clear the request cache before each test so results don't bleed across tests.
RequestCache.clear_all_namespaces()

def test_cache_hit_on_repeated_call(self):
"""Repeated calls with identical arguments should only invoke the enforcer once."""
username = "alice"
action = permissions.DELETE_LIBRARY.identifier
scope = "lib:Org1:math_101"

with patch("openedx_authz.api.users.is_subject_allowed") as mock_enforce:
mock_enforce.return_value = True
first = is_user_allowed(user_external_key=username, action_external_key=action, scope_external_key=scope)
second = is_user_allowed(user_external_key=username, action_external_key=action, scope_external_key=scope)

self.assertTrue(first)
self.assertEqual(first, second)
# The underlying enforcement should have been invoked only once; the second call is served from cache.
self.assertEqual(mock_enforce.call_count, 1)

def test_cache_miss_on_different_scope(self):
"""Different scope arguments must produce independent cache entries."""
username = "alice"
action = permissions.DELETE_LIBRARY.identifier

with patch("openedx_authz.api.users.is_subject_allowed") as mock_enforce:
mock_enforce.return_value = True
is_user_allowed(
user_external_key=username, action_external_key=action, scope_external_key="lib:Org1:math_101"
)
is_user_allowed(
user_external_key=username, action_external_key=action, scope_external_key="lib:Org1:math_advanced"
)

# Each distinct scope key is a separate cache entry → two enforcer calls.
self.assertEqual(mock_enforce.call_count, 2)

def test_cache_cleared_after_role_mutation(self):
"""The cache must be invalidated when a role assignment changes within the same request."""
username = "alice"
action = permissions.DELETE_LIBRARY.identifier
scope = "lib:Org1:math_101"

# alice has the permission before unassignment
before = is_user_allowed(user_external_key=username, action_external_key=action, scope_external_key=scope)
self.assertTrue(before)

# Unassigning roles should clear the cache so the next call goes to the enforcer.
unassign_all_roles_from_user(user_external_key=username)

after = is_user_allowed(user_external_key=username, action_external_key=action, scope_external_key=scope)
self.assertFalse(after)


@ddt
class TestValidateUsersAPI(UserAssignmentsSetupMixin):
"""Test suite for validate_users API function - focused on business logic."""
Expand Down