diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 591c5d92..3128bf50 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,19 @@ Unreleased * +0.11.1 - 2025-10-29 +******************** + +Changed +======= + +* Refactor to get permissions' scopes instead of role. + +Fixed +===== + +* Use correct content library toggle to check if Content Library V2 is enabled. + 0.11.0 - 2025-10-29 ******************** diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index ea0dd1d0..6844d0bd 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "0.11.0" +__version__ = "0.11.1" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__)) diff --git a/openedx_authz/api/data.py b/openedx_authz/api/data.py index 7b954d12..5fa39c05 100644 --- a/openedx_authz/api/data.py +++ b/openedx_authz/api/data.py @@ -656,6 +656,30 @@ def identifier(self) -> str: """ return self.action.external_key + def __eq__(self, other: "PermissionData") -> bool: + """Compare permissions based on their action identifier. + + Two PermissionData instances are considered equal if they have the same action's + external_key and effect. + + 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='allow') + >>> perm1 == perm2 # True - same action and effect + True + >>> perm1 in [perm2] # Uses __eq__ + True + """ + if self.action is None or other.action is None: + return False + return self.action.external_key == other.action.external_key and self.effect == other.effect + def __str__(self): """Human readable string representation of the permission and its effect.""" return f"{self.action} - {self.effect}" diff --git a/openedx_authz/api/roles.py b/openedx_authz/api/roles.py index fb0c996c..2c0f02a7 100644 --- a/openedx_authz/api/roles.py +++ b/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: @@ -381,20 +381,23 @@ def get_subjects_for_role_in_scope(role: RoleData, scope: ScopeData) -> list[Sub ] -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 diff --git a/openedx_authz/api/users.py b/openedx_authz/api/users.py index c42b59ef..7b1f58a7 100644 --- a/openedx_authz/api/users.py +++ b/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, @@ -34,6 +34,7 @@ "get_user_role_assignments_for_role_in_scope", "get_all_user_role_assignments_in_scope", "is_user_allowed", + "get_scopes_for_user_and_permission", "get_users_for_role_in_scope", ] @@ -205,17 +206,20 @@ def get_users_for_role_in_scope(role_external_key: str, scope_external_key: str) 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)), ) diff --git a/openedx_authz/constants/permissions.py b/openedx_authz/constants/permissions.py index f108d6d7..376fb63f 100644 --- a/openedx_authz/constants/permissions.py +++ b/openedx_authz/constants/permissions.py @@ -1,6 +1,7 @@ """ Default permission constants. """ + from openedx_authz.api.data import ActionData, PermissionData # Content Library Permissions diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index 4676aba0..2c4193d6 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -11,7 +11,15 @@ 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, @@ -19,7 +27,7 @@ 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, @@ -509,23 +517,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 = roles.LIBRARY_AUTHOR.external_key - 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_data( (roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_101", {"liam"}),