From e06afbb55a04acd39e8f1e3eb9e552c8422dfa79 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 17 Oct 2025 15:23:51 -0500 Subject: [PATCH 01/10] refactor: replace FastEnforcer with SyncedEnforcer --- openedx_authz/api/permissions.py | 1 - openedx_authz/api/roles.py | 7 ------- openedx_authz/engine/enforcer.py | 17 +++++++++-------- openedx_authz/settings/common.py | 1 + openedx_authz/settings/test.py | 1 + 5 files changed, 11 insertions(+), 16 deletions(-) diff --git a/openedx_authz/api/permissions.py b/openedx_authz/api/permissions.py index a1cc3de0..18cb93bc 100644 --- a/openedx_authz/api/permissions.py +++ b/openedx_authz/api/permissions.py @@ -65,7 +65,6 @@ def is_subject_allowed( bool: True if the subject has the specified permission in the scope, False otherwise. """ enforcer = AuthzEnforcer.get_enforcer() - enforcer.load_policy() return enforcer.enforce( subject.namespaced_key, action.namespaced_key, scope.namespaced_key ) diff --git a/openedx_authz/api/roles.py b/openedx_authz/api/roles.py index e415a747..937a0502 100644 --- a/openedx_authz/api/roles.py +++ b/openedx_authz/api/roles.py @@ -116,7 +116,6 @@ def get_permissions_for_active_roles_in_scope( permissions and scopes. """ enforcer = AuthzEnforcer.get_enforcer() - enforcer.load_policy() filtered_policy = enforcer.get_filtered_grouping_policy( GroupingPolicyIndex.SCOPE.value, scope.namespaced_key ) @@ -149,7 +148,6 @@ def get_role_definitions_in_scope(scope: ScopeData) -> list[RoleData]: list[Role]: A list of roles. """ enforcer = AuthzEnforcer.get_enforcer() - enforcer.load_policy() policy_filtered = enforcer.get_filtered_policy( PolicyIndex.SCOPE.value, scope.namespaced_key ) @@ -196,7 +194,6 @@ def get_all_roles_in_scope(scope: ScopeData) -> list[list[str]]: list[list[str]]: A list of policies in the specified scope. """ enforcer = AuthzEnforcer.get_enforcer() - enforcer.load_policy() return enforcer.get_filtered_grouping_policy( GroupingPolicyIndex.SCOPE.value, scope.namespaced_key ) @@ -216,7 +213,6 @@ def assign_role_to_subject_in_scope( bool: True if the role was assigned successfully, False otherwise. """ enforcer = AuthzEnforcer.get_enforcer() - enforcer.load_policy() return enforcer.add_role_for_user_in_domain( subject.namespaced_key, role.namespaced_key, @@ -251,7 +247,6 @@ def unassign_role_from_subject_in_scope( bool: True if the role was unassigned successfully, False otherwise. """ enforcer = AuthzEnforcer.get_enforcer() - enforcer.load_policy() return enforcer.delete_roles_for_user_in_domain( subject.namespaced_key, role.namespaced_key, scope.namespaced_key ) @@ -311,7 +306,6 @@ def get_subject_role_assignments_in_scope( list[RoleAssignmentData]: A list of role assignments for the subject in the scope. """ enforcer = AuthzEnforcer.get_enforcer() - enforcer.load_policy() # TODO: we still need to get the remaining data for the role like email, etc role_assignments = [] for namespaced_key in enforcer.get_roles_for_user_in_domain( @@ -412,6 +406,5 @@ def get_subjects_for_role(role: RoleData) -> list[SubjectData]: list[SubjectData]: A list of subjects assigned to the specified role. """ enforcer = AuthzEnforcer.get_enforcer() - enforcer.load_policy() policies = enforcer.get_filtered_grouping_policy(GroupingPolicyIndex.ROLE.value, role.namespaced_key) return [SubjectData(namespaced_key=policy[GroupingPolicyIndex.SUBJECT.value]) for policy in policies] diff --git a/openedx_authz/engine/enforcer.py b/openedx_authz/engine/enforcer.py index 76b4d12c..b6d2a88f 100644 --- a/openedx_authz/engine/enforcer.py +++ b/openedx_authz/engine/enforcer.py @@ -18,7 +18,7 @@ import logging -from casbin import FastEnforcer +from casbin import SyncedEnforcer from casbin_adapter.enforcer import initialize_enforcer from django.conf import settings @@ -29,7 +29,7 @@ class AuthzEnforcer: - """Singleton class to manage the Casbin FastEnforcer instance. + """Singleton class to manage the Casbin SyncedEnforcer instance. Ensures a single enforcer instance is created safely and configured with the ExtendedAdapter and Redis watcher for policy management and synchronization. @@ -60,20 +60,20 @@ def __new__(cls): return cls._enforcer @classmethod - def get_enforcer(cls) -> FastEnforcer: + def get_enforcer(cls) -> SyncedEnforcer: """Get the enforcer instance, creating it if needed. Returns: - FastEnforcer: The singleton enforcer instance. + SyncedEnforcer: The singleton enforcer instance. """ if cls._enforcer is None: cls._enforcer = cls._initialize_enforcer() return cls._enforcer @staticmethod - def _initialize_enforcer() -> FastEnforcer: + def _initialize_enforcer() -> SyncedEnforcer: """ - Create and configure the Casbin FastEnforcer instance. + Create and configure the Casbin SyncedEnforcer instance. This method initializes the FastEnforcer with the ExtendedAdapter for database policy storage and sets up the Redis watcher for real-time @@ -81,7 +81,7 @@ def _initialize_enforcer() -> FastEnforcer: the enforcer with the specified database alias from settings. Returns: - FastEnforcer: Configured Casbin enforcer with adapter and watcher + SyncedEnforcer: Configured Casbin enforcer with adapter and watcher """ db_alias = getattr(settings, "CASBIN_DB_ALIAS", "default") @@ -95,7 +95,8 @@ def _initialize_enforcer() -> FastEnforcer: raise adapter = ExtendedAdapter() - enforcer = FastEnforcer(settings.CASBIN_MODEL, adapter, enable_log=True) + enforcer = SyncedEnforcer(settings.CASBIN_MODEL, adapter) + enforcer.start_auto_load_policy(settings.CASBIN_AUTO_LOAD_POLICY_INTERVAL) enforcer.enable_auto_save(True) if not Watcher: diff --git a/openedx_authz/settings/common.py b/openedx_authz/settings/common.py index 98620ca7..2d58e48f 100644 --- a/openedx_authz/settings/common.py +++ b/openedx_authz/settings/common.py @@ -25,6 +25,7 @@ def plugin_settings(settings): ROOT_DIRECTORY, "engine", "config", "model.conf" ) settings.CASBIN_WATCHER_ENABLED = False + settings.CASBIN_AUTO_LOAD_POLICY_INTERVAL = 5 # TODO: Replace with a more dynamic configuration # Redis host and port are temporarily loaded here for the MVP settings.REDIS_HOST = "redis" diff --git a/openedx_authz/settings/test.py b/openedx_authz/settings/test.py index 2cb8c038..4a3243f3 100644 --- a/openedx_authz/settings/test.py +++ b/openedx_authz/settings/test.py @@ -69,6 +69,7 @@ def plugin_settings(settings): # pylint: disable=unused-argument # Casbin configuration CASBIN_MODEL = os.path.join(ROOT_DIRECTORY, "engine", "config", "model.conf") +CASBIN_AUTO_LOAD_POLICY_INTERVAL = 5 CASBIN_WATCHER_ENABLED = False REDIS_HOST = "redis" REDIS_PORT = 6379 From d3fc44b48acd535b007317e875cb42aed3c324b4 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 17 Oct 2025 15:36:15 -0500 Subject: [PATCH 02/10] docs: update response format documentation in RoleUserAPIView --- openedx_authz/rest_api/v1/views.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index f938f599..a977a9f0 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -178,9 +178,7 @@ class RoleUserAPIView(APIView): **Response Format (GET)** - Returns HTTP 200 OK with: - - .. code-block:: json + Returns HTTP 200 OK with:: { "count": 2, @@ -204,9 +202,7 @@ class RoleUserAPIView(APIView): **Response Format (PUT)** - Returns HTTP 207 Multi-Status with: - - .. code-block:: json + Returns HTTP 207 Multi-Status with:: { "completed": [{"user_identifier": "john_doe", "status": "role_added"}], @@ -215,9 +211,7 @@ class RoleUserAPIView(APIView): **Response Format (DELETE)** - Returns HTTP 207 Multi-Status with: - - .. code-block:: json + Returns HTTP 207 Multi-Status with:: { "completed": [{"user_identifier": "john_doe", "status": "role_removed"}], @@ -233,9 +227,7 @@ class RoleUserAPIView(APIView): GET /api/authz/v1/roles/users/?scope=lib:DemoX:CSPROB&search=john&roles=library_admin - PUT /api/authz/v1/roles/users/ - - .. code-block:: json + PUT /api/authz/v1/roles/users/ :: { "role": "library_admin", From 2ff8f97a84d4ce7195bb3dfb63b061b8d4094c0a Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 17 Oct 2025 15:36:26 -0500 Subject: [PATCH 03/10] test: enhance test isolation by adding setup and teardown methods in BaseRolesTestCase --- openedx_authz/tests/api/test_roles.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index f2b898cc..2833c33b 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -95,6 +95,16 @@ def setUpClass(cls): super().setUpClass() cls._seed_database_with_policies() + def setUp(self): + """Set up test environment.""" + super().setUp() + AuthzEnforcer.get_enforcer().load_policy() # Load policies before each test to simulate fresh start + + def tearDown(self): + """Clean up after each test to ensure isolation.""" + super().tearDown() + AuthzEnforcer.get_enforcer().clear_policy() # Clear policies after each test to ensure isolation + class RolesTestSetupMixin(BaseRolesTestCase): """Test case with comprehensive role assignments for general roles testing.""" @@ -230,16 +240,6 @@ def setUpClass(cls): ] cls._assign_roles_to_users(assignments=assignments) - def setUp(self): - """Set up test environment.""" - super().setUp() - AuthzEnforcer.get_enforcer().load_policy() # Load policies before each test to simulate fresh start - - def tearDown(self): - """Clean up after each test to ensure isolation.""" - super().tearDown() - AuthzEnforcer.get_enforcer().clear_policy() # Clear policies after each test to ensure isolation - @ddt class TestRolesAPI(RolesTestSetupMixin): From be1119c3a45ef837946c56178bbf194794f95a8d Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 17 Oct 2025 15:50:18 -0500 Subject: [PATCH 04/10] docs: update example response formatting in PermissionValidationMeView and RoleListView --- openedx_authz/rest_api/v1/views.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index a977a9f0..fe769957 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -76,18 +76,14 @@ class PermissionValidationMeView(APIView): **Example Request** - POST /api/authz/v1/permissions/validate/me - - .. code-block:: json + POST /api/authz/v1/permissions/validate/me:: [ {"action": "edit_library", "scope": "lib:DemoX:CSPROB"}, {"action": "delete_library_content", "scope": "lib:OpenedX:CS50"} ] - **Example Response** - - .. code-block:: json + **Example Response**:: [ {"action": "edit_library", "scope": "lib:DemoX:CSPROB", "allowed": true}, @@ -396,9 +392,7 @@ class RoleListView(APIView): GET /api/authz/v1/roles/?scope=lib:OpenedX:CSPROB&page=1&page_size=10 - **Example Response** - - .. code-block:: json + **Example Response**:: { "count": 2, From a0e69919bdaa5db82410a0333e06e4dc6e6204ed Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 21 Oct 2025 19:37:55 -0500 Subject: [PATCH 05/10] test: add auto-load policy functionality tests --- openedx_authz/tests/test_enforcer.py | 102 ++++++++++++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/openedx_authz/tests/test_enforcer.py b/openedx_authz/tests/test_enforcer.py index cdc97037..6d2dccdc 100644 --- a/openedx_authz/tests/test_enforcer.py +++ b/openedx_authz/tests/test_enforcer.py @@ -5,14 +5,18 @@ that would be used in production environments. """ +import time + import casbin from ddt import data as ddt_data from ddt import ddt -from django.test import TestCase +from django.conf import settings +from django.test import TestCase, TransactionTestCase, override_settings from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.engine.filter import Filter from openedx_authz.engine.utils import migrate_policy_between_enforcers +from openedx_authz.tests.test_utils import make_action_key, make_role_key, make_scope_key, make_user_key class PolicyLoadingTestSetupMixin(TestCase): @@ -420,3 +424,99 @@ def test_multi_scope_filtering(self): total_count = len(global_enforcer.get_policy()) self.assertEqual(total_count, lib_count + course_count + org_count) + + +class TestAutoLoadPolicy(TransactionTestCase): + """Test cases for auto-load policy functionality. + + Uses TransactionTestCase to avoid database locking issues with SQLite + when testing concurrent access patterns. + """ + + def setUp(self): + """Set up test environment.""" + super().setUp() + AuthzEnforcer._enforcer = None # pylint: disable=protected-access + + def _seed_database_with_policies(self): + """Seed the database with policies from the policy file.""" + global_enforcer = AuthzEnforcer.get_enforcer() + global_enforcer.clear_policy() + + migrate_policy_between_enforcers( + source_enforcer=casbin.Enforcer( + "openedx_authz/engine/config/model.conf", + "openedx_authz/engine/config/authz.policy", + ), + target_enforcer=global_enforcer, + ) + global_enforcer.clear_policy() + + def _wait_for_auto_load(self) -> None: + """Wait for one auto-load cycle plus a small buffer. + + This uses the configured interval plus a buffer to ensure + the auto-load has completed. + """ + interval = settings.CASBIN_AUTO_LOAD_POLICY_INTERVAL + # Add 50% buffer to ensure auto-load completes + time.sleep(interval * 1.5) + + @override_settings(CASBIN_AUTO_LOAD_POLICY_INTERVAL=0.5) + def test_auto_load_policy_detects_changes(self): + """Test that policy changes are automatically detected without manual reload. + + This test verifies that the SyncedEnforcer's auto-load functionality + works correctly by: + 1. Setting a short auto-load interval (0.5 seconds) + 2. Seeding the database with policies + 3. Waiting for auto-load to populate the enforcer + 4. Adding a new policy via add_policy() (auto-saved to DB) + 5. Waiting for auto-load to detect and load the change + 6. Adding a role assignment via add_role_for_user_in_domain() + 7. Verifying both changes appear without manual reload + + Expected result: + - Seeded policies are automatically loaded from database + - New policies added via add_policy() appear after auto-load interval + - Role assignments added via add_role_for_user_in_domain() appear after auto-load interval + - No explicit load_policy() calls are needed + """ + global_enforcer = AuthzEnforcer.get_enforcer() + self._seed_database_with_policies() + + # Initial policy count should be 0 + initial_policy_count = len(global_enforcer.get_policy()) + self.assertEqual(initial_policy_count, 0) + self._wait_for_auto_load() + + # After auto-load, the default policies should be loaded + policies_after_auto_load = global_enforcer.get_policy() + self.assertGreater(len(policies_after_auto_load), initial_policy_count) + + # Add a new policy + new_policy = [ + make_role_key("fake_role"), + make_action_key("fake_action"), + make_scope_key("lib", "*"), + "allow", + ] + global_enforcer.add_policy(*new_policy) + self._wait_for_auto_load() + + # After auto-load, the new policy should be loaded + policies_after_auto_load = global_enforcer.get_policy() + self.assertIn(new_policy, policies_after_auto_load) + + # Add a new role assignment + new_assignment = [ + make_user_key("fake_user"), + make_role_key("fake_role"), + make_scope_key("lib", "lib:FakeOrg:FAKELIB"), + ] + global_enforcer.add_role_for_user_in_domain(*new_assignment) + self._wait_for_auto_load() + + # After auto-load, the new role assignment should be loaded + policies_after_auto_load = global_enforcer.get_grouping_policy() + self.assertIn(new_assignment, policies_after_auto_load) From fba5f3863198f9ba18a8ea49692a1c78605e9fa1 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Tue, 21 Oct 2025 19:43:53 -0500 Subject: [PATCH 06/10] style: format code --- openedx_authz/rest_api/v1/views.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index fe769957..3fb10759 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -111,13 +111,7 @@ def post(self, request: HttpRequest) -> Response: action = perm["action"] scope = perm["scope"] allowed = api.is_user_allowed(username, action, scope) - response_data.append( - { - "action": action, - "scope": scope, - "allowed": allowed, - } - ) + response_data.append({"action": action, "scope": scope, "allowed": allowed}) except ValueError as e: logger.error(f"Error validating permission for user {username}: {e}") return Response(data={"message": "Invalid scope format"}, status=status.HTTP_400_BAD_REQUEST) From 83a5c30c61131371719c2c551879daabc725981a Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 22 Oct 2025 11:10:09 -0500 Subject: [PATCH 07/10] fix: ensure CASBIN_AUTO_LOAD_POLICY_INTERVAL is set only if not already defined --- openedx_authz/settings/common.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openedx_authz/settings/common.py b/openedx_authz/settings/common.py index 2d58e48f..d870d684 100644 --- a/openedx_authz/settings/common.py +++ b/openedx_authz/settings/common.py @@ -25,7 +25,8 @@ def plugin_settings(settings): ROOT_DIRECTORY, "engine", "config", "model.conf" ) settings.CASBIN_WATCHER_ENABLED = False - settings.CASBIN_AUTO_LOAD_POLICY_INTERVAL = 5 + if not hasattr(settings, "CASBIN_AUTO_LOAD_POLICY_INTERVAL"): + settings.CASBIN_AUTO_LOAD_POLICY_INTERVAL = 5 # TODO: Replace with a more dynamic configuration # Redis host and port are temporarily loaded here for the MVP settings.REDIS_HOST = "redis" From 467f8e317b338356ad17a36707148e12173ef339 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 22 Oct 2025 11:17:57 -0500 Subject: [PATCH 08/10] docs: update CHANGELOG --- CHANGELOG.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ea9c849d..0c714611 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,7 +14,8 @@ Change Log Unreleased ********** -* +* Use a SyncedEnforcer with default auto load policy. + 0.5.0 - 2025-10-21 ****************** From 73f7d51b702ae781bea4aab5d7eaba5f2f75b479 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 22 Oct 2025 11:22:35 -0500 Subject: [PATCH 09/10] fix: reduce CASBIN_AUTO_LOAD_POLICY_INTERVAL from 5 to 1 in tests --- openedx_authz/settings/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx_authz/settings/test.py b/openedx_authz/settings/test.py index 4a3243f3..3cbc0a0d 100644 --- a/openedx_authz/settings/test.py +++ b/openedx_authz/settings/test.py @@ -69,7 +69,7 @@ def plugin_settings(settings): # pylint: disable=unused-argument # Casbin configuration CASBIN_MODEL = os.path.join(ROOT_DIRECTORY, "engine", "config", "model.conf") -CASBIN_AUTO_LOAD_POLICY_INTERVAL = 5 +CASBIN_AUTO_LOAD_POLICY_INTERVAL = 1 CASBIN_WATCHER_ENABLED = False REDIS_HOST = "redis" REDIS_PORT = 6379 From da44e9659e4dc7d8fe843b4142efd991252217bb Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Wed, 22 Oct 2025 11:37:37 -0500 Subject: [PATCH 10/10] test: add tearDownClass method to BaseRolesTestCase for cleanup of auto-load policy thread --- openedx_authz/tests/api/test_roles.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index 2833c33b..fe584614 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -95,6 +95,18 @@ def setUpClass(cls): super().setUpClass() cls._seed_database_with_policies() + @classmethod + def tearDownClass(cls): + """Clean up after all tests in the class. + + Stops the auto-load policy thread to prevent database locking issues + with SQLite during concurrent access. + """ + super().tearDownClass() + enforcer = AuthzEnforcer.get_enforcer() + if hasattr(enforcer, 'stop_auto_load_policy'): + enforcer.stop_auto_load_policy() + def setUp(self): """Set up test environment.""" super().setUp()