refactor: consider no filtering as first version

Author: mariajgrimaldi

openedx_authz/api/decorators.py

@@ -1,6 +1,8 @@
 """Decorators for the authorization public API."""
 from functools import wraps
 
+from django.conf import settings
+
 from openedx_authz.api.data import ScopeData
 from openedx_authz.engine.enforcer import enforcer
 from openedx_authz.engine.filter import Filter
@@ -35,8 +37,18 @@ def get_all_roles():
         def get_roles_in_scope(scope: ScopeData):
             return enforcer.get_filtered_roles(scope.namespaced_key)
     """
-    FILTER_DATA_CLASSES = {
+    FILTER_DATA_CLASSES = { # Consider empty for no filtering
         "scope": ScopeData,
+        # TODO: currently ALLOW_FILTERED_POLICY_LOADING is False to avoid partial policy loads. We can
+        # Allow filtering on scope (initially) once we have a CONF model that supports this so filtering is meaningful,
+        # consistent and doesn't lead to partial policy loads.
+        # We can consider this modeling to avoid partial loads and inconsistent states:
+        # 1. g only for user-role-scope bindings
+        # 2. p only for permission-role bindings
+        # 3. g2 only for role-role bindings
+        # 4. g3 only for permission grouping
+        # This way for a user we'd only need to load g ( filter only for the scope or user) , p, g2, g3 policies in each request
+        # The only filter binding would be g, the rest loads entirely to avoid not loading definitions.
     }
 
     def build_filter_from_args(args) -> Filter:
@@ -48,6 +60,10 @@ def build_filter_from_args(args) -> Filter:
         Returns:
             Filter: A Filter object populated with relevant filter values.
         """
+        # Fallback to no filtering in case of misbehavior
+        if settings.ALLOW_FILTERED_POLICY_LOADING is False:
+            return Filter()
+
         filter_obj = Filter()
         if not filter_on or filter_on not in FILTER_DATA_CLASSES:
             return filter_obj

openedx_authz/settings/common.py

@@ -30,3 +30,8 @@ def plugin_settings(settings):
     # Redis host and port are temporarily loaded here for the MVP
     settings.REDIS_HOST = "redis"
     settings.REDIS_PORT = 6379
+
+    # TODO: Set to True when we have a CONF model that supports filtered policy loading meaningfully
+    # to avoid partial policy loads and inconsistent states.
+    # See comments in api/decorators.py for more details.
+    settings.ALLOW_FILTERED_POLICY_LOADING = False

openedx_authz/tests/api/test_decorators.py

@@ -375,3 +375,51 @@ def test_decorator_with_different_scopes(
             )
         else:
             self.assertEqual(len(permissions), 0)
+
+    def test_decorator_with_permission_grouping(self):
+        """Test decorator behavior with permission grouping in policies.
+
+        For example:
+            - manage_library_team includes view_library_team through g2
+
+        This test verifies that when a user has edit permissions, they also implicitly have
+        delete permissions due to the permission grouping defined in the policy.
+
+        Expected result:
+            - Decorator loads filtered policies for the given scope
+            - User with manage_library_team role can also view_library_team
+            - Enforcer is cleared after execution
+        """
+        scope = ScopeData(external_key="lib:Org1:math_101")
+        subject = SubjectData(external_key="alice")
+
+        @manage_policy_lifecycle(filter_on="scope")
+        def check_grouped_permissions(scope_arg, subject_arg):
+            """Check if subject has grouped permissions in the given scope.
+
+            Expected scenario:
+            - Alice has library_admin role in lib:Org1:math_101
+            - library_admin role includes manage_library_team
+            - manage_library_team includes view_library_team through g2
+            """
+            can_manage_team = global_enforcer.enforce(
+                subject_arg.namespaced_key,
+                ActionData(external_key="manage_library_team").namespaced_key,
+                scope_arg.namespaced_key,
+            )
+
+            can_view_team = global_enforcer.enforce(
+                subject_arg.namespaced_key,
+                ActionData(external_key="view_library_team").namespaced_key,
+                scope_arg.namespaced_key,
+            )
+
+            return {
+                "can_manage_team": can_manage_team,
+                "can_view_team": can_view_team,
+            }
+
+        result = check_grouped_permissions(scope, subject)
+
+        self.assertTrue(result["can_manage_team"], "Alice should be able to manage library team")
+        self.assertTrue(result["can_view_team"], "Alice should be able to view library team due to grouping")