WIP: Experimenting with cache invalidation

Author: rodmgwgu

openedx_authz/api/roles.py

@@ -197,11 +197,14 @@ def assign_role_to_subject_in_scope(subject: SubjectData, role: RoleData, scope:
         bool: True if the role was assigned successfully, False otherwise.
     """
     enforcer = AuthzEnforcer.get_enforcer()
-    return enforcer.add_role_for_user_in_domain(
+    success = enforcer.add_role_for_user_in_domain(
         subject.namespaced_key,
         role.namespaced_key,
         scope.namespaced_key,
     )
+    # Invalidate policy cache to ensure changes are picked up
+    AuthzEnforcer.invalidate_policy_cache()
+    return success
 
 
 def batch_assign_role_to_subjects_in_scope(subjects: list[SubjectData], role: RoleData, scope: ScopeData) -> None:
@@ -227,7 +230,10 @@ def unassign_role_from_subject_in_scope(subject: SubjectData, role: RoleData, sc
         bool: True if the role was unassigned successfully, False otherwise.
     """
     enforcer = AuthzEnforcer.get_enforcer()
-    return enforcer.delete_roles_for_user_in_domain(subject.namespaced_key, role.namespaced_key, scope.namespaced_key)
+    success = enforcer.delete_roles_for_user_in_domain(subject.namespaced_key, role.namespaced_key, scope.namespaced_key)
+    # Invalidate policy cache to ensure changes are picked up
+    AuthzEnforcer.invalidate_policy_cache()
+    return success
 
 
 def batch_unassign_role_from_subjects_in_scope(subjects: list[SubjectData], role: RoleData, scope: ScopeData) -> None:

openedx_authz/engine/enforcer.py

@@ -16,10 +16,12 @@
 """
 
 import logging
+import time
 
 from casbin import SyncedEnforcer
 from casbin_adapter.enforcer import initialize_enforcer
 from django.conf import settings
+from django.core.cache import cache
 
 from openedx_authz.engine.adapter import ExtendedAdapter
 
@@ -62,7 +64,10 @@ class AuthzEnforcer:
     Any of the two approaches will yield the same singleton enforcer instance.
     """
 
+    CACHE_KEY = "authz_policy_last_modified_timestamp"
+
     _enforcer = None
+    _last_policy_load_timestamp = None
 
     def __new__(cls):
         """Singleton pattern to ensure a single enforcer instance."""
@@ -141,13 +146,57 @@ def configure_enforcer_auto_save_and_load(cls):
         auto_load_policy_interval = getattr(settings, "CASBIN_AUTO_LOAD_POLICY_INTERVAL", 0)
         auto_save_policy = getattr(settings, "CASBIN_AUTO_SAVE_POLICY", True)
 
-        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.")
+        # TODO: remove autoload in favor of cache invalidation?
+        # 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)
 
+    @classmethod
+    def load_policy_if_needed(cls):
+        """Load policy if the last load timestamp indicates it's needed.
+
+        This method checks if the policy needs to be reloaded comparing
+        the last load timestamp with the last modified timestamp in cache 
+        and reloads it if necessary.
+
+        Returns:
+            None
+        """
+        last_modified_timestamp = cache.get(cls.CACHE_KEY)
+
+        current_timestamp = time.time()
+
+        if last_modified_timestamp is None:
+            # No timestamp in cache; initialize it
+            cache.set(cls.CACHE_KEY, current_timestamp, None)
+            logger.info(f">>>> Initialized policy last modified timestamp in cache. {current_timestamp}")
+
+        if (
+            cls._last_policy_load_timestamp is None or
+            last_modified_timestamp > cls._last_policy_load_timestamp
+        ):
+            # Policy has been modified since last load; reload it
+            cls._enforcer.load_policy()
+            cls._last_policy_load_timestamp = current_timestamp
+            logger.info(f">>>> Reloaded policy at {current_timestamp}")
+
+    @classmethod
+    def invalidate_policy_cache(cls):
+        """Invalidate the current policy cache to force a reload on next check.
+
+        This method updates the last modified timestamp in the cache to
+        the current time, indicating that the policy has changed.
+
+        Returns:
+            None
+        """
+        current_timestamp = time.time()
+        cache.set(cls.CACHE_KEY, current_timestamp, None)
+        logger.info(f">>>> Invalidated policy cache at {current_timestamp}")
+
     @classmethod
     def get_enforcer(cls) -> SyncedEnforcer:
         """Get the enforcer instance, creating it if needed.
@@ -158,6 +207,9 @@ def get_enforcer(cls) -> SyncedEnforcer:
         if cls._enforcer is None:
             cls._enforcer = cls._initialize_enforcer()
 
+        # (re)load policy if needed
+        cls.load_policy_if_needed()
+
         # HACK: This code block will only be useful when in Ulmo to deactivate
         # the enforcer when the new library experience is disabled. It should be
         # removed for the next release cycle.

openedx_authz/rest_api/v1/permissions.py

@@ -183,7 +183,7 @@ def has_permission(self, request, view) -> bool:
         """
         if request.user.is_superuser or request.user.is_staff:
             return True
-        AuthzEnforcer.get_enforcer().load_policy()
+        # AuthzEnforcer.get_enforcer().load_policy()
         return self._get_permission_instance(request).has_permission(request, view)
 
     def has_object_permission(self, request, view, obj) -> bool:
@@ -200,7 +200,7 @@ def has_object_permission(self, request, view, obj) -> bool:
         """
         if request.user.is_superuser or request.user.is_staff:
             return True
-        AuthzEnforcer.get_enforcer().load_policy()
+        # AuthzEnforcer.get_enforcer().load_policy()
         return self._get_permission_instance(request).has_object_permission(request, view, obj)
 
 

openedx_authz/rest_api/v1/views.py

@@ -103,7 +103,7 @@ class PermissionValidationMeView(APIView):
     )
     def post(self, request: HttpRequest) -> Response:
         """Validate one or more permissions for the authenticated user."""
-        AuthzEnforcer.get_enforcer().load_policy()
+        # AuthzEnforcer.get_enforcer().load_policy()
 
         serializer = PermissionValidationSerializer(data=request.data, many=True)
         serializer.is_valid(raise_exception=True)