refactor: address quality issues

Author: mariajgrimaldi

openedx_authz/api/decorators.py

@@ -47,8 +47,8 @@ def get_roles_in_scope(scope: ScopeData):
         # 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.
+        # 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:

openedx_authz/api/roles.py

@@ -19,9 +19,9 @@
     ScopeData,
     SubjectData,
 )
+from openedx_authz.api.decorators import manage_policy_lifecycle
 from openedx_authz.api.permissions import get_permission_from_policy
 from openedx_authz.engine.enforcer import enforcer
-from openedx_authz.api.decorators import manage_policy_lifecycle
 
 __all__ = [
     "get_permissions_for_single_role",

openedx_authz/engine/adapter.py

@@ -18,7 +18,7 @@
 from casbin.persist import FilteredAdapter
 from casbin_adapter.adapter import Adapter
 from casbin_adapter.models import CasbinRule
-from django.db.models import QuerySet, Q
+from django.db.models import QuerySet
 
 from openedx_authz.engine.filter import Filter
 

openedx_authz/settings/test.py

@@ -57,3 +57,4 @@
 SECRET_KEY = "test-secret-key"
 CASBIN_WATCHER_ENABLED = False
 USE_TZ = True
+ALLOW_FILTERED_POLICY_LOADING = False

openedx_authz/tests/api/test_decorators.py

@@ -5,21 +5,15 @@
 """
 
 import casbin
+import pytest
 from ddt import data as ddt_data
 from ddt import ddt, unpack
+from django.conf import settings
 from django.test import TestCase
 
-from openedx_authz.api.data import (
-    ActionData,
-    RoleData,
-    ScopeData,
-    SubjectData,
-)
+from openedx_authz.api.data import ActionData, RoleData, ScopeData, SubjectData
 from openedx_authz.api.decorators import manage_policy_lifecycle
-from openedx_authz.api.roles import (
-    assign_role_to_subject_in_scope,
-    get_permissions_for_active_roles_in_scope,
-)
+from openedx_authz.api.roles import assign_role_to_subject_in_scope, get_permissions_for_active_roles_in_scope
 from openedx_authz.engine.enforcer import enforcer as global_enforcer
 from openedx_authz.engine.filter import Filter
 from openedx_authz.engine.utils import migrate_policy_between_enforcers
@@ -90,6 +84,7 @@ def tearDown(self):
         super().tearDown()
         global_enforcer.clear_policy()
 
+    @pytest.mark.skipif(settings.ALLOW_FILTERED_POLICY_LOADING is False, reason="Filtered policy loading not allowed")
     def test_decorator_filters_by_scope_and_clears(self):
         """Test decorator loads filtered policies by scope and clears after execution.
 
@@ -102,7 +97,7 @@ def test_decorator_filters_by_scope_and_clears(self):
         scope = ScopeData(external_key="lib:Org1:math_101")
 
         @manage_policy_lifecycle(filter_on="scope")
-        def get_policy_info(scope_arg):
+        def get_policy_info(scope_arg):  # pylint: disable=unused-argument
             policy_count = len(global_enforcer.get_policy())
             grouping_policy_count = len(global_enforcer.get_grouping_policy())
             return {
@@ -133,7 +128,7 @@ def test_decorator_loads_full_policy_without_filter(self):
         """
 
         @manage_policy_lifecycle(filter_on="scope")
-        def get_full_policy_count(some_arg):
+        def get_full_policy_count(some_arg):  # pylint: disable=unused-argument
             """Function that does not take a scope argument.
 
             This should cause the decorator to load the full policy.
@@ -161,7 +156,7 @@ def test_decorator_clears_policy_on_exception(self):
         """
 
         @manage_policy_lifecycle(filter_on="scope")
-        def failing_function(scope_arg):
+        def failing_function(scope_arg):  # pylint: disable=unused-argument
             """Function that raises an exception to test decorator cleanup."""
             if len(global_enforcer.get_policy()) >= 0:
                 raise ValueError("Intentional test exception")
@@ -174,7 +169,8 @@ def failing_function(scope_arg):
 
         self.assertEqual(str(context.exception), "Intentional test exception")
 
-    def test_decorator_with_enforcement_checks(self):
+    @pytest.mark.skipif(settings.ALLOW_FILTERED_POLICY_LOADING is False, reason="Filtered policy loading not allowed")
+    def test_decorator_with_enforcement_checks_with_filtered_loading(self):
         """Test that policies loaded by decorator enable correct enforcement decisions.
 
         Expected result:
@@ -235,6 +231,7 @@ def check_permissions(scope_arg, subject_arg):
         self.assertEqual(result["policy_count"], expected_policies)
         self.assertEqual(result["grouping_count"], expected_grouping)
 
+    @pytest.mark.skipif(settings.ALLOW_FILTERED_POLICY_LOADING is False, reason="Filtered policy loading not allowed")
     def test_decorator_enforcement_with_different_subjects(self):
         """Test enforcement with different subjects having different roles.