refactor: get permissions' scopes instead of role

Author: mariajgrimaldi

openedx_authz/api/data.py

@@ -366,6 +366,15 @@ def library_id(self) -> str:
         """
         return self.external_key
 
+    @property
+    def library_key(self) -> LibraryLocatorV2:
+        """The LibraryLocatorV2 object for the content library.
+
+        Returns:
+            LibraryLocatorV2: The library locator object.
+        """
+        return LibraryLocatorV2.from_string(self.library_id)
+
     @classmethod
     def validate_external_key(cls, external_key: str) -> bool:
         """Validate the external_key format for ContentLibraryData.
@@ -611,6 +620,33 @@ def identifier(self) -> str:
         """
         return self.action.external_key
 
+    def __eq__(self, other):
+        """Compare permissions based on their action identifier.
+
+        Two permissions are considered equal if they have the same action,
+        regardless of the effect. This allows checking if a role grants a
+        specific action using the 'in' operator.
+
+        Args:
+            other: Another PermissionData instance or any object.
+
+        Returns:
+            bool: True if the actions match, False otherwise.
+
+        Example:
+            >>> perm1 = PermissionData(action=ActionData(external_key='view'), effect='allow')
+            >>> perm2 = PermissionData(action=ActionData(external_key='view'), effect='deny')
+            >>> perm1 == perm2  # True - same action TODO: should we also consider the effect?
+            True
+            >>> perm1 in [perm2]  # Uses __eq__
+            True
+        """
+        if not isinstance(other, PermissionData):
+            return False
+        if self.action is None or other.action is None:
+            return False
+        return self.action.external_key == other.action.external_key
+
     def __str__(self):
         """Human readable string representation of the permission and its effect."""
         return f"{self.action} - {self.effect}"

openedx_authz/api/roles.py

@@ -37,7 +37,7 @@
     "get_subject_role_assignments_for_role_in_scope",
     "get_all_subject_role_assignments_in_scope",
     "get_subject_role_assignments",
-    "get_scopes_for_role_and_subject",
+    "get_scopes_for_subject_and_permission",
 ]
 
 # TODO: these are the concerns we still have to address:
@@ -376,20 +376,23 @@ def get_subjects_for_role(role: RoleData) -> list[SubjectData]:
     return [SubjectData(namespaced_key=policy[GroupingPolicyIndex.SUBJECT.value]) for policy in policies]
 
 
-def get_scopes_for_role_and_subject(role: RoleData, subject: SubjectData) -> list[ScopeData]:
-    """Get all the scopes where a specific subject is assigned a specific role.
+def get_scopes_for_subject_and_permission(
+    subject: SubjectData,
+    permission: PermissionData,
+) -> list[ScopeData]:
+    """Get all scopes where a specific subject has been assigned a specific permission via roles.
 
     Args:
-        role (RoleData): The role to filter scopes.
+        permission (PermissionData): The permission to filter scopes.
         subject (SubjectData): The subject to filter scopes.
 
     Returns:
-        list[ScopeData]: A list of scopes where the subject is assigned the specified role.
+        list[ScopeData]: A list of scopes where the subject is assigned the specified permission.
     """
-    enforcer = AuthzEnforcer.get_enforcer()
-    policies = enforcer.get_filtered_grouping_policy(GroupingPolicyIndex.SUBJECT.value, subject.namespaced_key)
-    return [
-        ScopeData(namespaced_key=policy[GroupingPolicyIndex.SCOPE.value])
-        for policy in policies
-        if policy[GroupingPolicyIndex.ROLE.value] == role.namespaced_key
-    ]
+    roles_for_subject = get_subject_role_assignments(subject)
+    scopes = []
+    for role_assignment in roles_for_subject:
+        for role in role_assignment.roles:
+            if permission in role.permissions:
+                scopes.append(role_assignment.scope)
+    return scopes

openedx_authz/api/users.py

@@ -9,14 +9,14 @@
 (e.g., 'user^john_doe').
 """
 
-from openedx_authz.api.data import ActionData, RoleAssignmentData, RoleData, ScopeData, UserData
+from openedx_authz.api.data import ActionData, PermissionData, RoleAssignmentData, RoleData, ScopeData, UserData
 from openedx_authz.api.permissions import is_subject_allowed
 from openedx_authz.api.roles import (
     assign_role_to_subject_in_scope,
     batch_assign_role_to_subjects_in_scope,
     batch_unassign_role_from_subjects_in_scope,
     get_all_subject_role_assignments_in_scope,
-    get_scopes_for_role_and_subject,
+    get_scopes_for_subject_and_permission,
     get_subject_role_assignments,
     get_subject_role_assignments_for_role_in_scope,
     get_subject_role_assignments_in_scope,
@@ -35,6 +35,7 @@
     "get_all_user_role_assignments_in_scope",
     "is_user_allowed",
     "get_users_for_role",
+    "get_scopes_for_user_and_permission",
 ]
 
 
@@ -201,17 +202,20 @@ def get_users_for_role(role_external_key: str) -> list[UserData]:
     return [UserData(namespaced_key=user.namespaced_key) for user in users]
 
 
-def get_scopes_for_role_and_user(role_external_key: str, user_external_key: str) -> list[ScopeData]:
-    """Get all scopes where a specific user has been assigned a specific role.
+def get_scopes_for_user_and_permission(
+    user_external_key: str,
+    action_external_key: str,
+) -> list[ScopeData]:
+    """Get all scopes where a specific user is assigned a specific permission.
 
     Args:
-        role_external_key (str): The role to filter scopes (e.g., 'instructor').
         user_external_key (str): ID of the user (e.g., 'john_doe').
+        action_external_key (str): The action to filter scopes (e.g., 'view', 'edit').
 
     Returns:
-        list[ScopeData]: A list of scopes where the user has the specified role.
+        list[ScopeData]: A list of scopes where the user is assigned the specified permission.
     """
-    return get_scopes_for_role_and_subject(
-        RoleData(external_key=role_external_key),
+    return get_scopes_for_subject_and_permission(
         UserData(external_key=user_external_key),
+        PermissionData(action=ActionData(external_key=action_external_key)),
     )

openedx_authz/tests/api/test_roles.py

@@ -10,15 +10,23 @@
 from ddt import ddt, unpack
 from django.test import TestCase
 
-from openedx_authz.api.data import ContentLibraryData, RoleAssignmentData, RoleData, ScopeData, SubjectData
+from openedx_authz.api.data import (
+    ActionData,
+    ContentLibraryData,
+    PermissionData,
+    RoleAssignmentData,
+    RoleData,
+    ScopeData,
+    SubjectData,
+)
 from openedx_authz.api.roles import (
     assign_role_to_subject_in_scope,
     batch_assign_role_to_subjects_in_scope,
     get_all_subject_role_assignments_in_scope,
     get_permissions_for_active_roles_in_scope,
     get_permissions_for_single_role,
     get_role_definitions_in_scope,
-    get_scopes_for_role_and_subject,
+    get_scopes_for_subject_and_permission,
     get_subject_role_assignments,
     get_subject_role_assignments_for_role_in_scope,
     get_subject_role_assignments_in_scope,
@@ -506,23 +514,108 @@ def test_get_role_assignments_in_scope(self, role_name, scope_name, expected_cou
 
         self.assertEqual(len(role_assignments), expected_count)
 
-    def test_get_scopes_for_role_and_subject(self):
-        """Test retrieving scopes for a given role and subject.
+    @ddt_data(
+        # Test case: alice with 'view_library' permission (has library_admin in math_101)
+        (
+            "alice",
+            "view_library",
+            ["lib:Org1:math_101"],
+        ),
+        # Test case: alice with 'publish_library_content' permission (admin grants publish)
+        (
+            "alice",
+            "publish_library_content",
+            ["lib:Org1:math_101"],
+        ),
+        # Test case: alice with 'delete_library' permission (admin grants delete)
+        (
+            "alice",
+            "delete_library",
+            ["lib:Org1:math_101"],
+        ),
+        # Test case: bob with 'view_library' permission (has library_author in history_201)
+        (
+            "bob",
+            "view_library",
+            ["lib:Org1:history_201"],
+        ),
+        # Test case: bob with 'publish_library_content' permission (author grants publish)
+        (
+            "bob",
+            "publish_library_content",
+            ["lib:Org1:history_201"],
+        ),
+        # Test case: bob with 'delete_library' permission (author does NOT grant delete)
+        (
+            "bob",
+            "delete_library",
+            [],
+        ),
+        # Test case: carol with 'view_library' permission (has library_contributor in science_301)
+        (
+            "carol",
+            "view_library",
+            ["lib:Org1:science_301"],
+        ),
+        # Test case: carol with 'publish_library_content' permission (contributor does NOT grant publish)
+        (
+            "carol",
+            "publish_library_content",
+            [],
+        ),
+        # Test case: dave with 'view_library' permission (has library_user in english_101)
+        (
+            "dave",
+            "view_library",
+            ["lib:Org1:english_101"],
+        ),
+        # Test case: dave with 'publish_library_content' permission (user does NOT grant publish)
+        (
+            "dave",
+            "publish_library_content",
+            [],
+        ),
+        # Test case: liam with 'view_library' permission (has library_author in 3 art libraries)
+        (
+            "liam",
+            "view_library",
+            ["lib:Org4:art_101", "lib:Org4:art_201", "lib:Org4:art_301"],
+        ),
+        # Test case: non-existent user
+        (
+            "nonexistent",
+            "view_library",
+            [],
+        ),
+    )
+    @unpack
+    def test_get_scopes_for_subject_and_permission(self, subject_name, action_name, expected_scope_names):
+        """Test retrieving scopes where a subject has a specific permission.
+
+        This tests the get_scopes_for_subject_and_permission function which
+        returns all scopes where a subject has been granted a specific permission
+        through their role assignments.
+
+        Args:
+            subject_name: The external key of the subject (e.g., 'alice')
+            action_name: The action to check (e.g., 'view', 'edit', 'delete')
+            expected_scope_names: List of expected scope external keys
 
         Expected result:
-            - The scopes associated with the specified role and subject are correctly retrieved.
+            - Returns all scopes where the subject has roles that grant the permission
+            - Returns empty list if subject has no roles with that permission
         """
-        role_name = "library_author"
-        subject_name = "liam"
-        expected_scopes = {"lib:Org4:art_101", "lib:Org4:art_201", "lib:Org4:art_301"}
+        subject = SubjectData(external_key=subject_name)
+        permission = PermissionData(action=ActionData(external_key=action_name))
 
-        scopes = get_scopes_for_role_and_subject(
-            RoleData(external_key=role_name),
-            SubjectData(external_key=subject_name),
-        )
+        scopes = get_scopes_for_subject_and_permission(subject, permission)
+
+        # Extract scope external keys for comparison
+        actual_scope_names = [scope.external_key for scope in scopes]
 
-        scope_names = {scope.external_key for scope in scopes}
-        self.assertEqual(scope_names, expected_scopes)
+        self.assertEqual(len(actual_scope_names), len(expected_scope_names))
+        for expected_scope in expected_scope_names:
+            self.assertIn(expected_scope, actual_scope_names)
 
 
 @ddt