From b84369527b71541ceca18329fc38b15c365478ff Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Tue, 28 Oct 2025 11:41:55 -0500 Subject: [PATCH 1/4] fix: add constants and improve test class --- openedx_authz/constants/__init__.py | 0 openedx_authz/constants/permissions.py | 16 +++ openedx_authz/constants/roles.py | 148 +++++++++++++++++++++++++ openedx_authz/tests/api/test_roles.py | 15 +-- openedx_authz/tests/api/test_users.py | 2 +- openedx_authz/tests/constants.py | 141 ----------------------- 6 files changed, 173 insertions(+), 149 deletions(-) create mode 100644 openedx_authz/constants/__init__.py create mode 100644 openedx_authz/constants/permissions.py create mode 100644 openedx_authz/constants/roles.py delete mode 100644 openedx_authz/tests/constants.py diff --git a/openedx_authz/constants/__init__.py b/openedx_authz/constants/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/openedx_authz/constants/permissions.py b/openedx_authz/constants/permissions.py new file mode 100644 index 00000000..27268457 --- /dev/null +++ b/openedx_authz/constants/permissions.py @@ -0,0 +1,16 @@ +""" +Default permission constants. +""" + +# Content Library Permissions +VIEW_LIBRARY = "view_library" +MANAGE_LIBRARY_TAGS = "manage_library_tags" +DELETE_LIBRARY = "delete_library" +EDIT_LIBRARY_CONTENT = "edit_library_content" +PUBLISH_LIBRARY_CONTENT = "publish_library_content" +REUSE_LIBRARY_CONTENT = "reuse_library_content" +VIEW_LIBRARY_TEAM = "view_library_team" +MANAGE_LIBRARY_TEAM = "manage_library_team" +CREATE_LIBRARY_COLLECTION = "create_library_collection" +EDIT_LIBRARY_COLLECTION = "edit_library_collection" +DELETE_LIBRARY_COLLECTION = "delete_library_collection" diff --git a/openedx_authz/constants/roles.py b/openedx_authz/constants/roles.py new file mode 100644 index 00000000..2c320a94 --- /dev/null +++ b/openedx_authz/constants/roles.py @@ -0,0 +1,148 @@ +""" +Default roles and their associated permissions. +""" + +from openedx_authz.api.data import ActionData, PermissionData +from openedx_authz.constants import permissions + +# Library Roles +LIBRARY_ADMIN = "library_admin" +LIBRARY_AUTHOR = "library_author" +LIBRARY_CONTRIBUTOR = "library_contributor" +LIBRARY_USER = "library_user" + +LIST_LIBRARY_ADMIN_PERMISSIONS = [ + PermissionData( + action=ActionData(external_key=permissions.VIEW_LIBRARY), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.MANAGE_LIBRARY_TAGS), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.DELETE_LIBRARY), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.EDIT_LIBRARY_CONTENT), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.PUBLISH_LIBRARY_CONTENT), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.REUSE_LIBRARY_CONTENT), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.VIEW_LIBRARY_TEAM), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.MANAGE_LIBRARY_TEAM), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.CREATE_LIBRARY_COLLECTION), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.EDIT_LIBRARY_COLLECTION), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.DELETE_LIBRARY_COLLECTION), + effect="allow", + ), +] + +LIST_LIBRARY_AUTHOR_PERMISSIONS = [ + PermissionData( + action=ActionData(external_key=permissions.VIEW_LIBRARY), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.MANAGE_LIBRARY_TAGS), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.EDIT_LIBRARY_CONTENT), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.PUBLISH_LIBRARY_CONTENT), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.REUSE_LIBRARY_CONTENT), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.VIEW_LIBRARY_TEAM), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.CREATE_LIBRARY_COLLECTION), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.EDIT_LIBRARY_COLLECTION), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.DELETE_LIBRARY_COLLECTION), + effect="allow", + ), +] + +LIST_LIBRARY_CONTRIBUTOR_PERMISSIONS = [ + PermissionData( + action=ActionData(external_key=permissions.VIEW_LIBRARY), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.MANAGE_LIBRARY_TAGS), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.EDIT_LIBRARY_CONTENT), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.REUSE_LIBRARY_CONTENT), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.VIEW_LIBRARY_TEAM), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.CREATE_LIBRARY_COLLECTION), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.EDIT_LIBRARY_COLLECTION), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.DELETE_LIBRARY_COLLECTION), + effect="allow", + ), +] + +LIST_LIBRARY_USER_PERMISSIONS = [ + PermissionData( + action=ActionData(external_key=permissions.VIEW_LIBRARY), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.REUSE_LIBRARY_CONTENT), + effect="allow", + ), + PermissionData( + action=ActionData(external_key=permissions.VIEW_LIBRARY_TEAM), + effect="allow", + ), +] diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index 900755b5..9c9c2e7d 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -6,6 +6,7 @@ """ import casbin +import pkg_resources from ddt import data as ddt_data from ddt import ddt, unpack from django.test import TestCase @@ -25,14 +26,14 @@ get_subjects_for_role_in_scope, unassign_role_from_subject_in_scope, ) -from openedx_authz.engine.enforcer import AuthzEnforcer -from openedx_authz.engine.utils import migrate_policy_between_enforcers -from openedx_authz.tests.constants import ( +from openedx_authz.constants.roles import ( LIST_LIBRARY_ADMIN_PERMISSIONS, LIST_LIBRARY_AUTHOR_PERMISSIONS, LIST_LIBRARY_CONTRIBUTOR_PERMISSIONS, LIST_LIBRARY_USER_PERMISSIONS, ) +from openedx_authz.engine.enforcer import AuthzEnforcer +from openedx_authz.engine.utils import migrate_policy_between_enforcers class BaseRolesTestCase(TestCase): @@ -52,11 +53,11 @@ def _seed_database_with_policies(cls): """ global_enforcer = AuthzEnforcer.get_enforcer() global_enforcer.load_policy() + model_path = pkg_resources.resource_filename("openedx_authz.engine", "config/model.conf") + policy_path = pkg_resources.resource_filename("openedx_authz.engine", "config/authz.policy") + migrate_policy_between_enforcers( - source_enforcer=casbin.Enforcer( - "openedx_authz/engine/config/model.conf", - "openedx_authz/engine/config/authz.policy", - ), + source_enforcer=casbin.Enforcer(model_path, policy_path), target_enforcer=global_enforcer, ) global_enforcer.clear_policy() # Clear to simulate fresh start for each test diff --git a/openedx_authz/tests/api/test_users.py b/openedx_authz/tests/api/test_users.py index 7f11fc1c..7c57f47a 100644 --- a/openedx_authz/tests/api/test_users.py +++ b/openedx_authz/tests/api/test_users.py @@ -14,8 +14,8 @@ is_user_allowed, unassign_role_from_user, ) +from openedx_authz.constants.roles import LIST_LIBRARY_ADMIN_PERMISSIONS, LIST_LIBRARY_AUTHOR_PERMISSIONS from openedx_authz.tests.api.test_roles import RolesTestSetupMixin -from openedx_authz.tests.constants import LIST_LIBRARY_ADMIN_PERMISSIONS, LIST_LIBRARY_AUTHOR_PERMISSIONS class UserAssignmentsSetupMixin(RolesTestSetupMixin): diff --git a/openedx_authz/tests/constants.py b/openedx_authz/tests/constants.py deleted file mode 100644 index afb54e21..00000000 --- a/openedx_authz/tests/constants.py +++ /dev/null @@ -1,141 +0,0 @@ -""" -Constants for library roles and permissions tests. -""" - -from openedx_authz.api.data import ActionData, PermissionData - -LIST_LIBRARY_ADMIN_PERMISSIONS = [ - PermissionData( - action=ActionData(external_key="view_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="reuse_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="view_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library_collection"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library_collection"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_collection"), - effect="allow", - ), -] - -LIST_LIBRARY_AUTHOR_PERMISSIONS = [ - PermissionData( - action=ActionData(external_key="view_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="reuse_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="view_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library_collection"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library_collection"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_collection"), - effect="allow", - ), -] - -LIST_LIBRARY_CONTRIBUTOR_PERMISSIONS = [ - PermissionData( - action=ActionData(external_key="view_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="reuse_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="view_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library_collection"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library_collection"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_collection"), - effect="allow", - ), -] - -LIST_LIBRARY_USER_PERMISSIONS = [ - PermissionData( - action=ActionData(external_key="view_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="reuse_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="view_library_team"), - effect="allow", - ), -] From 7a44fa104ab9db2acdf511d8d198a9eb2815aa85 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Tue, 28 Oct 2025 13:09:47 -0500 Subject: [PATCH 2/4] refactor: use the new constants --- openedx_authz/rest_api/v1/views.py | 9 +- openedx_authz/tests/api/test_data.py | 11 +- openedx_authz/tests/api/test_roles.py | 215 +++++++++++---------- openedx_authz/tests/api/test_users.py | 63 +++--- openedx_authz/tests/rest_api/test_views.py | 83 ++++---- openedx_authz/tests/test_commands.py | 5 +- openedx_authz/tests/test_enforcement.py | 13 +- openedx_authz/tests/test_engine_utils.py | 45 ++--- 8 files changed, 226 insertions(+), 218 deletions(-) diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index fc72398d..bb16789b 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -15,6 +15,7 @@ from rest_framework.views import APIView from openedx_authz import api +from openedx_authz.constants import permissions from openedx_authz.rest_api.data import RoleOperationError, RoleOperationStatus from openedx_authz.rest_api.decorators import authz_permissions, view_auth_classes from openedx_authz.rest_api.utils import ( @@ -250,7 +251,7 @@ class RoleUserAPIView(APIView): status.HTTP_401_UNAUTHORIZED: "The user is not authenticated or does not have the required permissions", }, ) - @authz_permissions(["view_library_team"]) + @authz_permissions([permissions.VIEW_LIBRARY_TEAM]) def get(self, request: HttpRequest) -> Response: """Retrieve all users with role assignments within a specific scope.""" serializer = ListUsersInRoleWithScopeSerializer(data=request.query_params) @@ -277,7 +278,7 @@ def get(self, request: HttpRequest) -> Response: status.HTTP_401_UNAUTHORIZED: "The user is not authenticated or does not have the required permissions", }, ) - @authz_permissions(["manage_library_team"]) + @authz_permissions([permissions.MANAGE_LIBRARY_TEAM]) def put(self, request: HttpRequest) -> Response: """Assign multiple users to a specific role within a scope.""" serializer = AddUsersToRoleWithScopeSerializer(data=request.data) @@ -324,7 +325,7 @@ def put(self, request: HttpRequest) -> Response: status.HTTP_401_UNAUTHORIZED: "The user is not authenticated or does not have the required permissions", }, ) - @authz_permissions(["manage_library_team"]) + @authz_permissions([permissions.MANAGE_LIBRARY_TEAM]) def delete(self, request: HttpRequest) -> Response: """Remove multiple users from a specific role within a scope.""" serializer = RemoveUsersFromRoleWithScopeSerializer(data=request.query_params) @@ -427,7 +428,7 @@ class RoleListView(APIView): status.HTTP_401_UNAUTHORIZED: "The user is not authenticated or does not have the required permissions", }, ) - @authz_permissions(["view_library_team"]) + @authz_permissions([permissions.VIEW_LIBRARY_TEAM]) def get(self, request: HttpRequest) -> Response: """Retrieve all roles and their permissions for a specific scope.""" serializer = ListRolesWithScopeSerializer(data=request.query_params) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index 2925e6b3..fb087105 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -17,6 +17,7 @@ SubjectData, UserData, ) +from openedx_authz.constants import permissions, roles @ddt @@ -372,7 +373,7 @@ def test_user_data_str_and_repr(self, external_key, expected_str, expected_repr) @data( ("read", "Read", "act^read"), ("write", "Write", "act^write"), - ("delete_library", "Delete Library", "act^delete_library"), + (permissions.DELETE_LIBRARY, "Delete Library", "act^delete_library"), ("edit_content", "Edit Content", "act^edit_content"), ) @unpack @@ -413,7 +414,7 @@ def test_scope_data_str_and_repr(self, external_key, expected_str, expected_repr @data( ("instructor", "Instructor", "role^instructor"), - ("library_admin", "Library Admin", "role^library_admin"), + (roles.LIBRARY_ADMIN, "Library Admin", "role^library_admin"), ("course_staff", "Course Staff", "role^course_staff"), ) @unpack @@ -454,7 +455,7 @@ def test_role_data_str_with_permissions(self): ("read", "allow", "Read - allow", "act^read => allow"), ("write", "deny", "Write - deny", "act^write => deny"), ( - "delete_library", + permissions.DELETE_LIBRARY, "allow", "Delete Library - allow", "act^delete_library => allow", @@ -485,7 +486,7 @@ def test_role_assignment_data_str(self): """ user = UserData(external_key="john_doe") role1 = RoleData(external_key="instructor") - role2 = RoleData(external_key="library_admin") + role2 = RoleData(external_key=roles.LIBRARY_ADMIN) scope = ContentLibraryData(external_key="lib:DemoX:CSPROB") assignment = RoleAssignmentData(subject=user, roles=[role1, role2], scope=scope) @@ -502,7 +503,7 @@ def test_role_assignment_data_repr(self): """ user = UserData(external_key="john_doe") role1 = RoleData(external_key="instructor") - role2 = RoleData(external_key="library_admin") + role2 = RoleData(external_key=roles.LIBRARY_ADMIN) scope = ContentLibraryData(external_key="lib:DemoX:CSPROB") assignment = RoleAssignmentData(subject=user, roles=[role1, role2], scope=scope) diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index 9c9c2e7d..089cea4a 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -26,6 +26,7 @@ get_subjects_for_role_in_scope, unassign_role_from_subject_in_scope, ) +from openedx_authz.constants import roles from openedx_authz.constants.roles import ( LIST_LIBRARY_ADMIN_PERMISSIONS, LIST_LIBRARY_AUTHOR_PERMISSIONS, @@ -125,123 +126,123 @@ def setUpClass(cls): # Basic library roles from authz.policy { "subject_name": "alice", - "role_name": "library_admin", + "role_name": roles.LIBRARY_ADMIN, "scope_name": "lib:Org1:math_101", }, { "subject_name": "bob", - "role_name": "library_author", + "role_name": roles.LIBRARY_AUTHOR, "scope_name": "lib:Org1:history_201", }, { "subject_name": "carol", - "role_name": "library_contributor", + "role_name": roles.LIBRARY_CONTRIBUTOR, "scope_name": "lib:Org1:science_301", }, { "subject_name": "dave", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org1:english_101", }, # Multi-role assignments - same subject with different roles in different libraries { "subject_name": "eve", - "role_name": "library_admin", + "role_name": roles.LIBRARY_ADMIN, "scope_name": "lib:Org2:physics_401", }, { "subject_name": "eve", - "role_name": "library_author", + "role_name": roles.LIBRARY_AUTHOR, "scope_name": "lib:Org2:chemistry_501", }, { "subject_name": "eve", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org2:biology_601", }, # Multiple subjects with same role in same scope { "subject_name": "grace", - "role_name": "library_contributor", + "role_name": roles.LIBRARY_CONTRIBUTOR, "scope_name": "lib:Org1:math_advanced", }, { "subject_name": "heidi", - "role_name": "library_contributor", + "role_name": roles.LIBRARY_CONTRIBUTOR, "scope_name": "lib:Org1:math_advanced", }, # Hierarchical scope assignments - different specificity levels { "subject_name": "ivy", - "role_name": "library_admin", + "role_name": roles.LIBRARY_ADMIN, "scope_name": "lib:Org3:cs_101", }, { "subject_name": "jack", - "role_name": "library_author", + "role_name": roles.LIBRARY_AUTHOR, "scope_name": "lib:Org3:cs_101", }, { "subject_name": "kate", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org3:cs_101", }, # Edge case: same user, same role, different scopes { "subject_name": "liam", - "role_name": "library_author", + "role_name": roles.LIBRARY_AUTHOR, "scope_name": "lib:Org4:art_101", }, { "subject_name": "liam", - "role_name": "library_author", + "role_name": roles.LIBRARY_AUTHOR, "scope_name": "lib:Org4:art_201", }, { "subject_name": "liam", - "role_name": "library_author", + "role_name": roles.LIBRARY_AUTHOR, "scope_name": "lib:Org4:art_301", }, # Mixed permission levels across libraries for comprehensive testing { "subject_name": "maya", - "role_name": "library_admin", + "role_name": roles.LIBRARY_ADMIN, "scope_name": "lib:Org5:economics_101", }, { "subject_name": "noah", - "role_name": "library_contributor", + "role_name": roles.LIBRARY_CONTRIBUTOR, "scope_name": "lib:Org5:economics_101", }, { "subject_name": "olivia", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org5:economics_101", }, # Complex multi-library, multi-role scenario { "subject_name": "peter", - "role_name": "library_admin", + "role_name": roles.LIBRARY_ADMIN, "scope_name": "lib:Org6:project_alpha", }, { "subject_name": "peter", - "role_name": "library_author", + "role_name": roles.LIBRARY_AUTHOR, "scope_name": "lib:Org6:project_beta", }, { "subject_name": "peter", - "role_name": "library_contributor", + "role_name": roles.LIBRARY_CONTRIBUTOR, "scope_name": "lib:Org6:project_gamma", }, { "subject_name": "peter", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org6:project_delta", }, { "subject_name": "frank", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org6:project_epsilon", }, ] @@ -271,22 +272,22 @@ class TestRolesAPI(RolesTestSetupMixin): @ddt_data( # Library Admin role with actual permissions from authz.policy ( - "library_admin", + roles.LIBRARY_ADMIN, LIST_LIBRARY_ADMIN_PERMISSIONS, ), # Library Author role with actual permissions from authz.policy ( - "library_author", + roles.LIBRARY_AUTHOR, LIST_LIBRARY_AUTHOR_PERMISSIONS, ), # Library Contributor role with actual permissions from authz.policy ( - "library_contributor", + roles.LIBRARY_CONTRIBUTOR, LIST_LIBRARY_CONTRIBUTOR_PERMISSIONS, ), # Library User role with minimal permissions ( - "library_user", + roles.LIBRARY_USER, LIST_LIBRARY_USER_PERMISSIONS, ), # Non existent role @@ -310,19 +311,19 @@ def test_get_permissions_for_roles(self, role_name, expected_permissions): @ddt_data( # Role assigned to multiple users in different scopes ( - "library_user", + roles.LIBRARY_USER, "lib:Org1:english_101", LIST_LIBRARY_USER_PERMISSIONS, ), # Role assigned to single user in single scope ( - "library_author", + roles.LIBRARY_AUTHOR, "lib:Org1:history_201", LIST_LIBRARY_AUTHOR_PERMISSIONS, ), # Role assigned to single user in multiple scopes ( - "library_admin", + roles.LIBRARY_ADMIN, "lib:Org1:math_101", LIST_LIBRARY_ADMIN_PERMISSIONS, ), @@ -349,10 +350,10 @@ def test_get_permissions_for_active_role_in_specific_scope(self, role_name, scop ( "*", { - "library_admin", - "library_author", - "library_contributor", - "library_user", + roles.LIBRARY_ADMIN, + roles.LIBRARY_AUTHOR, + roles.LIBRARY_CONTRIBUTOR, + roles.LIBRARY_USER, }, ), ) @@ -376,27 +377,27 @@ def test_get_roles_in_scope(self, scope_name, expected_roles): self.assertEqual(role_names, expected_roles) @ddt_data( - ("alice", "lib:Org1:math_101", {"library_admin"}), - ("bob", "lib:Org1:history_201", {"library_author"}), - ("carol", "lib:Org1:science_301", {"library_contributor"}), - ("dave", "lib:Org1:english_101", {"library_user"}), - ("eve", "lib:Org2:physics_401", {"library_admin"}), - ("eve", "lib:Org2:chemistry_501", {"library_author"}), - ("eve", "lib:Org2:biology_601", {"library_user"}), - ("grace", "lib:Org1:math_advanced", {"library_contributor"}), - ("ivy", "lib:Org3:cs_101", {"library_admin"}), - ("jack", "lib:Org3:cs_101", {"library_author"}), - ("kate", "lib:Org3:cs_101", {"library_user"}), - ("liam", "lib:Org4:art_101", {"library_author"}), - ("liam", "lib:Org4:art_201", {"library_author"}), - ("liam", "lib:Org4:art_301", {"library_author"}), - ("maya", "lib:Org5:economics_101", {"library_admin"}), - ("noah", "lib:Org5:economics_101", {"library_contributor"}), - ("olivia", "lib:Org5:economics_101", {"library_user"}), - ("peter", "lib:Org6:project_alpha", {"library_admin"}), - ("peter", "lib:Org6:project_beta", {"library_author"}), - ("peter", "lib:Org6:project_gamma", {"library_contributor"}), - ("peter", "lib:Org6:project_delta", {"library_user"}), + ("alice", "lib:Org1:math_101", {roles.LIBRARY_ADMIN}), + ("bob", "lib:Org1:history_201", {roles.LIBRARY_AUTHOR}), + ("carol", "lib:Org1:science_301", {roles.LIBRARY_CONTRIBUTOR}), + ("dave", "lib:Org1:english_101", {roles.LIBRARY_USER}), + ("eve", "lib:Org2:physics_401", {roles.LIBRARY_ADMIN}), + ("eve", "lib:Org2:chemistry_501", {roles.LIBRARY_AUTHOR}), + ("eve", "lib:Org2:biology_601", {roles.LIBRARY_USER}), + ("grace", "lib:Org1:math_advanced", {roles.LIBRARY_CONTRIBUTOR}), + ("ivy", "lib:Org3:cs_101", {roles.LIBRARY_ADMIN}), + ("jack", "lib:Org3:cs_101", {roles.LIBRARY_AUTHOR}), + ("kate", "lib:Org3:cs_101", {roles.LIBRARY_USER}), + ("liam", "lib:Org4:art_101", {roles.LIBRARY_AUTHOR}), + ("liam", "lib:Org4:art_201", {roles.LIBRARY_AUTHOR}), + ("liam", "lib:Org4:art_301", {roles.LIBRARY_AUTHOR}), + ("maya", "lib:Org5:economics_101", {roles.LIBRARY_ADMIN}), + ("noah", "lib:Org5:economics_101", {roles.LIBRARY_CONTRIBUTOR}), + ("olivia", "lib:Org5:economics_101", {roles.LIBRARY_USER}), + ("peter", "lib:Org6:project_alpha", {roles.LIBRARY_ADMIN}), + ("peter", "lib:Org6:project_beta", {roles.LIBRARY_AUTHOR}), + ("peter", "lib:Org6:project_gamma", {roles.LIBRARY_CONTRIBUTOR}), + ("peter", "lib:Org6:project_delta", {roles.LIBRARY_USER}), ("non_existent_user", "lib:Org1:math_101", set()), ("alice", "lib:Org999:non_existent_scope", set()), ("non_existent_user", "lib:Org999:non_existent_scope", set()), @@ -420,7 +421,7 @@ def test_get_subject_role_assignments_in_scope(self, subject_name, scope_name, e "alice", [ RoleData( - external_key="library_admin", + external_key=roles.LIBRARY_ADMIN, permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, ), ], @@ -429,15 +430,15 @@ def test_get_subject_role_assignments_in_scope(self, subject_name, scope_name, e "eve", [ RoleData( - external_key="library_admin", + external_key=roles.LIBRARY_ADMIN, permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, ), RoleData( - external_key="library_author", + external_key=roles.LIBRARY_AUTHOR, permissions=LIST_LIBRARY_AUTHOR_PERMISSIONS, ), RoleData( - external_key="library_user", + external_key=roles.LIBRARY_USER, permissions=LIST_LIBRARY_USER_PERMISSIONS, ), ], @@ -446,7 +447,7 @@ def test_get_subject_role_assignments_in_scope(self, subject_name, scope_name, e "frank", [ RoleData( - external_key="library_user", + external_key=roles.LIBRARY_USER, permissions=LIST_LIBRARY_USER_PERMISSIONS, ), ], @@ -470,29 +471,29 @@ def test_get_all_role_assignments_scopes(self, subject_name, expected_roles): self.assertTrue(found, f"Expected role {expected_role} not found in assignments") @ddt_data( - ("library_admin", "lib:Org1:math_101", 1), - ("library_author", "lib:Org1:history_201", 1), - ("library_contributor", "lib:Org1:science_301", 1), - ("library_user", "lib:Org1:english_101", 1), - ("library_admin", "lib:Org2:physics_401", 1), - ("library_author", "lib:Org2:chemistry_501", 1), - ("library_user", "lib:Org2:biology_601", 1), - ("library_contributor", "lib:Org1:math_advanced", 2), - ("library_admin", "lib:Org3:cs_101", 1), - ("library_author", "lib:Org3:cs_101", 1), - ("library_user", "lib:Org3:cs_101", 1), - ("library_author", "lib:Org4:art_101", 1), - ("library_author", "lib:Org4:art_201", 1), - ("library_author", "lib:Org4:art_301", 1), - ("library_admin", "lib:Org5:economics_101", 1), - ("library_contributor", "lib:Org5:economics_101", 1), - ("library_user", "lib:Org5:economics_101", 1), - ("library_admin", "lib:Org6:project_alpha", 1), - ("library_author", "lib:Org6:project_beta", 1), - ("library_contributor", "lib:Org6:project_gamma", 1), - ("library_user", "lib:Org6:project_delta", 1), + (roles.LIBRARY_ADMIN, "lib:Org1:math_101", 1), + (roles.LIBRARY_AUTHOR, "lib:Org1:history_201", 1), + (roles.LIBRARY_CONTRIBUTOR, "lib:Org1:science_301", 1), + (roles.LIBRARY_USER, "lib:Org1:english_101", 1), + (roles.LIBRARY_ADMIN, "lib:Org2:physics_401", 1), + (roles.LIBRARY_AUTHOR, "lib:Org2:chemistry_501", 1), + (roles.LIBRARY_USER, "lib:Org2:biology_601", 1), + (roles.LIBRARY_CONTRIBUTOR, "lib:Org1:math_advanced", 2), + (roles.LIBRARY_ADMIN, "lib:Org3:cs_101", 1), + (roles.LIBRARY_AUTHOR, "lib:Org3:cs_101", 1), + (roles.LIBRARY_USER, "lib:Org3:cs_101", 1), + (roles.LIBRARY_AUTHOR, "lib:Org4:art_101", 1), + (roles.LIBRARY_AUTHOR, "lib:Org4:art_201", 1), + (roles.LIBRARY_AUTHOR, "lib:Org4:art_301", 1), + (roles.LIBRARY_ADMIN, "lib:Org5:economics_101", 1), + (roles.LIBRARY_CONTRIBUTOR, "lib:Org5:economics_101", 1), + (roles.LIBRARY_USER, "lib:Org5:economics_101", 1), + (roles.LIBRARY_ADMIN, "lib:Org6:project_alpha", 1), + (roles.LIBRARY_AUTHOR, "lib:Org6:project_beta", 1), + (roles.LIBRARY_CONTRIBUTOR, "lib:Org6:project_gamma", 1), + (roles.LIBRARY_USER, "lib:Org6:project_delta", 1), ("non_existent_role", "sc:any_library", 0), - ("library_admin", "sc:non_existent_scope", 0), + (roles.LIBRARY_ADMIN, "sc:non_existent_scope", 0), ("non_existent_role", "sc:non_existent_scope", 0), ) @unpack @@ -514,7 +515,7 @@ def test_get_scopes_for_role_and_subject(self): Expected result: - The scopes associated with the specified role and subject are correctly retrieved. """ - role_name = "library_author" + role_name = roles.LIBRARY_AUTHOR subject_name = "liam" expected_scopes = {"lib:Org4:art_101", "lib:Org4:art_201", "lib:Org4:art_301"} @@ -527,11 +528,11 @@ def test_get_scopes_for_role_and_subject(self): self.assertEqual(scope_names, expected_scopes) @ddt_data( - ("library_author", "lib:Org4:art_101", {"liam"}), - ("library_author", "lib:Org4:art_201", {"liam"}), - ("library_author", "lib:Org4:art_301", {"liam"}), + (roles.LIBRARY_AUTHOR, "lib:Org4:art_101", {"liam"}), + (roles.LIBRARY_AUTHOR, "lib:Org4:art_201", {"liam"}), + (roles.LIBRARY_AUTHOR, "lib:Org4:art_301", {"liam"}), ("non_existent_role", "lib:Org4:art_101", set()), - ("library_author", "sc:non_existent_scope", set()), + (roles.LIBRARY_AUTHOR, "sc:non_existent_scope", set()), ("non_existent_role", "sc:non_existent_scope", set()), ) @unpack @@ -560,24 +561,24 @@ class TestRoleAssignmentAPI(RolesTestSetupMixin): """ @ddt_data( - (["mary", "john"], "library_user", "sc:batch_test", True), + (["mary", "john"], roles.LIBRARY_USER, "sc:batch_test", True), ( ["paul", "diana", "lila"], - "library_contributor", + roles.LIBRARY_CONTRIBUTOR, "lib:Org1:math_advanced", True, ), - (["sarina", "ty"], "library_author", "lib:Org4:art_101", True), - (["fran", "bob"], "library_admin", "lib:Org3:cs_101", True), + (["sarina", "ty"], roles.LIBRARY_AUTHOR, "lib:Org4:art_101", True), + (["fran", "bob"], roles.LIBRARY_ADMIN, "lib:Org3:cs_101", True), ( ["anna", "tom", "jerry"], - "library_user", + roles.LIBRARY_USER, "lib:Org1:history_201", True, ), - ("joe", "library_contributor", "lib:Org1:science_301", False), - ("nina", "library_author", "lib:Org1:english_101", False), - ("oliver", "library_admin", "lib:Org1:math_101", False), + ("joe", roles.LIBRARY_CONTRIBUTOR, "lib:Org1:science_301", False), + ("nina", roles.LIBRARY_AUTHOR, "lib:Org1:english_101", False), + ("oliver", roles.LIBRARY_ADMIN, "lib:Org1:math_101", False), ) @unpack def test_batch_assign_role_to_subjects_in_scope(self, subject_names, role, scope_name, batch): @@ -618,24 +619,24 @@ def test_batch_assign_role_to_subjects_in_scope(self, subject_names, role, scope self.assertIn(role, role_names) @ddt_data( - (["mary", "john"], "library_user", "sc:batch_test", True), + (["mary", "john"], roles.LIBRARY_USER, "sc:batch_test", True), ( ["paul", "diana", "lila"], - "library_contributor", + roles.LIBRARY_CONTRIBUTOR, "lib:Org1:math_advanced", True, ), - (["sarina", "ty"], "library_author", "lib:Org4:art_101", True), - (["fran", "bob"], "library_admin", "lib:Org3:cs_101", True), + (["sarina", "ty"], roles.LIBRARY_AUTHOR, "lib:Org4:art_101", True), + (["fran", "bob"], roles.LIBRARY_ADMIN, "lib:Org3:cs_101", True), ( ["anna", "tom", "jerry"], - "library_user", + roles.LIBRARY_USER, "lib:Org1:history_201", True, ), - ("joe", "library_contributor", "lib:Org1:science_301", False), - ("nina", "library_author", "lib:Org1:english_101", False), - ("oliver", "library_admin", "lib:Org1:math_101", False), + ("joe", roles.LIBRARY_CONTRIBUTOR, "lib:Org1:science_301", False), + ("nina", roles.LIBRARY_AUTHOR, "lib:Org1:english_101", False), + ("oliver", roles.LIBRARY_ADMIN, "lib:Org1:math_101", False), ) @unpack def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_name, batch): @@ -680,7 +681,7 @@ def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_na subject=SubjectData(external_key="alice"), roles=[ RoleData( - external_key="library_admin", + external_key=roles.LIBRARY_ADMIN, permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, ) ], @@ -695,7 +696,7 @@ def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_na subject=SubjectData(external_key="bob"), roles=[ RoleData( - external_key="library_author", + external_key=roles.LIBRARY_AUTHOR, permissions=LIST_LIBRARY_AUTHOR_PERMISSIONS, ) ], @@ -710,7 +711,7 @@ def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_na subject=SubjectData(external_key="carol"), roles=[ RoleData( - external_key="library_contributor", + external_key=roles.LIBRARY_CONTRIBUTOR, permissions=LIST_LIBRARY_CONTRIBUTOR_PERMISSIONS, ) ], @@ -725,7 +726,7 @@ def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_na subject=SubjectData(external_key="dave"), roles=[ RoleData( - external_key="library_user", + external_key=roles.LIBRARY_USER, permissions=LIST_LIBRARY_USER_PERMISSIONS, ) ], diff --git a/openedx_authz/tests/api/test_users.py b/openedx_authz/tests/api/test_users.py index 7c57f47a..23cb2757 100644 --- a/openedx_authz/tests/api/test_users.py +++ b/openedx_authz/tests/api/test_users.py @@ -14,6 +14,7 @@ is_user_allowed, unassign_role_from_user, ) +from openedx_authz.constants import permissions, roles from openedx_authz.constants.roles import LIST_LIBRARY_ADMIN_PERMISSIONS, LIST_LIBRARY_AUTHOR_PERMISSIONS from openedx_authz.tests.api.test_roles import RolesTestSetupMixin @@ -51,10 +52,10 @@ class TestUserRoleAssignments(UserAssignmentsSetupMixin): """Test suite for user-role assignment API functions.""" @data( - ("john", "library_admin", "lib:Org1:math_101", False), - ("jane", "library_user", "lib:Org1:english_101", False), - (["mary", "charlie"], "library_contributor", "lib:Org1:science_301", True), - (["david", "sarah"], "library_author", "lib:Org1:history_201", True), + ("john", roles.LIBRARY_ADMIN, "lib:Org1:math_101", False), + ("jane", roles.LIBRARY_USER, "lib:Org1:english_101", False), + (["mary", "charlie"], roles.LIBRARY_CONTRIBUTOR, "lib:Org1:science_301", True), + (["david", "sarah"], roles.LIBRARY_AUTHOR, "lib:Org1:history_201", True), ) @unpack def test_assign_role_to_user_in_scope(self, username, role, scope_name, batch): @@ -80,10 +81,10 @@ def test_assign_role_to_user_in_scope(self, username, role, scope_name, batch): self.assertIn(role, role_names) @data( - (["grace"], "library_contributor", "lib:Org1:math_advanced", True), - (["liam", "maya"], "library_author", "lib:Org4:art_101", True), - ("alice", "library_admin", "lib:Org1:math_101", False), - ("bob", "library_author", "lib:Org1:history_201", False), + (["grace"], roles.LIBRARY_CONTRIBUTOR, "lib:Org1:math_advanced", True), + (["liam", "maya"], roles.LIBRARY_AUTHOR, "lib:Org4:art_101", True), + ("alice", roles.LIBRARY_ADMIN, "lib:Org1:math_101", False), + ("bob", roles.LIBRARY_AUTHOR, "lib:Org1:history_201", False), ) @unpack def test_unassign_role_from_user(self, username, role, scope_name, batch): @@ -110,9 +111,9 @@ def test_unassign_role_from_user(self, username, role, scope_name, batch): self.assertNotIn(role, role_names) @data( - ("eve", {"library_admin", "library_author", "library_user"}), - ("alice", {"library_admin"}), - ("liam", {"library_author"}), + ("eve", {roles.LIBRARY_ADMIN, roles.LIBRARY_AUTHOR, roles.LIBRARY_USER}), + ("alice", {roles.LIBRARY_ADMIN}), + ("liam", {roles.LIBRARY_AUTHOR}), ) @unpack def test_get_user_role_assignments(self, username, expected_roles): @@ -128,10 +129,10 @@ def test_get_user_role_assignments(self, username, expected_roles): self.assertEqual(assigned_role_names, expected_roles) @data( - ("alice", "lib:Org1:math_101", {"library_admin"}), - ("bob", "lib:Org1:history_201", {"library_author"}), - ("eve", "lib:Org2:physics_401", {"library_admin"}), - ("grace", "lib:Org1:math_advanced", {"library_contributor"}), + ("alice", "lib:Org1:math_101", {roles.LIBRARY_ADMIN}), + ("bob", "lib:Org1:history_201", {roles.LIBRARY_AUTHOR}), + ("eve", "lib:Org2:physics_401", {roles.LIBRARY_ADMIN}), + ("grace", "lib:Org1:math_advanced", {roles.LIBRARY_CONTRIBUTOR}), ) @unpack def test_get_user_role_assignments_in_scope(self, username, scope_name, expected_roles): @@ -147,9 +148,9 @@ def test_get_user_role_assignments_in_scope(self, username, scope_name, expected self.assertEqual(role_names, expected_roles) @data( - ("library_admin", "lib:Org1:math_101", {"alice"}), - ("library_author", "lib:Org1:history_201", {"bob"}), - ("library_contributor", "lib:Org1:math_advanced", {"grace", "heidi"}), + (roles.LIBRARY_ADMIN, "lib:Org1:math_101", {"alice"}), + (roles.LIBRARY_AUTHOR, "lib:Org1:history_201", {"bob"}), + (roles.LIBRARY_CONTRIBUTOR, "lib:Org1:math_advanced", {"grace", "heidi"}), ) @unpack def test_get_user_role_assignments_for_role_in_scope(self, role_name, scope_name, expected_users): @@ -175,7 +176,7 @@ def test_get_user_role_assignments_for_role_in_scope(self, role_name, scope_name subject=UserData(external_key="alice"), roles=[ RoleData( - external_key="library_admin", + external_key=roles.LIBRARY_ADMIN, permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, ) ], @@ -190,7 +191,7 @@ def test_get_user_role_assignments_for_role_in_scope(self, role_name, scope_name subject=UserData(external_key="bob"), roles=[ RoleData( - external_key="library_author", + external_key=roles.LIBRARY_AUTHOR, permissions=LIST_LIBRARY_AUTHOR_PERMISSIONS, ) ], @@ -205,7 +206,7 @@ def test_get_user_role_assignments_for_role_in_scope(self, role_name, scope_name subject=UserData(external_key="eve"), roles=[ RoleData( - external_key="library_admin", + external_key=roles.LIBRARY_ADMIN, permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, ) ], @@ -234,16 +235,16 @@ class TestUserPermissions(UserAssignmentsSetupMixin): """Test suite for user permission API functions.""" @data( - ("alice", "delete_library", "lib:Org1:math_101", True), - ("bob", "publish_library_content", "lib:Org1:history_201", True), - ("eve", "manage_library_team", "lib:Org2:physics_401", True), - ("grace", "edit_library_content", "lib:Org1:math_advanced", True), - ("heidi", "create_library_collection", "lib:Org1:math_advanced", True), - ("charlie", "delete_library", "lib:Org1:science_301", False), - ("david", "publish_library_content", "lib:Org1:history_201", False), - ("mallory", "manage_library_team", "lib:Org1:math_101", False), - ("oscar", "edit_library_content", "lib:Org4:art_101", False), - ("peggy", "create_library_collection", "lib:Org2:physics_401", False), + ("alice", permissions.DELETE_LIBRARY, "lib:Org1:math_101", True), + ("bob", permissions.PUBLISH_LIBRARY_CONTENT, "lib:Org1:history_201", True), + ("eve", permissions.MANAGE_LIBRARY_TEAM, "lib:Org2:physics_401", True), + ("grace", permissions.EDIT_LIBRARY_CONTENT, "lib:Org1:math_advanced", True), + ("heidi", permissions.CREATE_LIBRARY_COLLECTION, "lib:Org1:math_advanced", True), + ("charlie", permissions.DELETE_LIBRARY, "lib:Org1:science_301", False), + ("david", permissions.PUBLISH_LIBRARY_CONTENT, "lib:Org1:history_201", False), + ("mallory", permissions.MANAGE_LIBRARY_TEAM, "lib:Org1:math_101", False), + ("oscar", permissions.EDIT_LIBRARY_CONTENT, "lib:Org4:art_101", False), + ("peggy", permissions.CREATE_LIBRARY_COLLECTION, "lib:Org2:physics_401", False), ) @unpack def test_is_user_allowed(self, username, action, scope_name, expected_result): diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index 390a4ffc..ff5d6b5f 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -16,6 +16,7 @@ from openedx_authz import api from openedx_authz.api.users import assign_role_to_user_in_scope +from openedx_authz.constants import permissions, roles from openedx_authz.rest_api.data import RoleOperationError, RoleOperationStatus from openedx_authz.rest_api.v1.permissions import DynamicScopePermission from openedx_authz.tests.api.test_roles import BaseRolesTestCase @@ -68,48 +69,48 @@ def setUpClass(cls): # Assign roles to admin users { "subject_name": "admin_1", - "role_name": "library_admin", + "role_name": roles.LIBRARY_ADMIN, "scope_name": "lib:Org1:LIB1", }, { "subject_name": "admin_2", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org2:LIB2", }, { "subject_name": "admin_3", - "role_name": "library_admin", + "role_name": roles.LIBRARY_ADMIN, "scope_name": "lib:Org3:LIB3", }, # Assign roles to regular users { "subject_name": "regular_1", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org1:LIB1", }, { "subject_name": "regular_2", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org1:LIB1", }, { "subject_name": "regular_3", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org2:LIB2", }, { "subject_name": "regular_4", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org2:LIB2", }, { "subject_name": "regular_5", - "role_name": "library_admin", + "role_name": roles.LIBRARY_ADMIN, "scope_name": "lib:Org3:LIB3", }, { "subject_name": "regular_6", - "role_name": "library_author", + "role_name": roles.LIBRARY_AUTHOR, "scope_name": "lib:Org3:LIB3", }, { @@ -119,7 +120,7 @@ def setUpClass(cls): }, { "subject_name": "regular_8", - "role_name": "library_user", + "role_name": roles.LIBRARY_USER, "scope_name": "lib:Org3:LIB3", }, ] @@ -164,16 +165,16 @@ def setUp(self): @data( # Single permission - allowed - ([{"action": "view_library", "scope": "lib:Org1:LIB1"}], [True]), + ([{"action": permissions.VIEW_LIBRARY, "scope": "lib:Org1:LIB1"}], [True]), # Single permission - denied (scope not assigned to user) - ([{"action": "view_library", "scope": "lib:Org2:LIB2"}], [False]), + ([{"action": permissions.VIEW_LIBRARY, "scope": "lib:Org2:LIB2"}], [False]), # # Single permission - denied (action not assigned to user) ([{"action": "edit_library", "scope": "lib:Org1:LIB1"}], [False]), # # Multiple permissions - mixed results ( [ - {"action": "view_library", "scope": "lib:Org1:LIB1"}, - {"action": "view_library", "scope": "lib:Org2:LIB2"}, + {"action": permissions.VIEW_LIBRARY, "scope": "lib:Org1:LIB1"}, + {"action": permissions.VIEW_LIBRARY, "scope": "lib:Org2:LIB2"}, {"action": "edit_library", "scope": "lib:Org1:LIB1"}, ], [True, False, False], @@ -304,23 +305,23 @@ def setUp(self): ({"search": "@example.com"}, 3), ({"search": "nonexistent@example.com"}, 0), # Search by single role - ({"roles": "library_admin"}, 1), - ({"roles": "library_author"}, 0), - ({"roles": "library_user"}, 2), + ({"roles": roles.LIBRARY_ADMIN}, 1), + ({"roles": roles.LIBRARY_AUTHOR}, 0), + ({"roles": roles.LIBRARY_USER}, 2), # Search by multiple roles ({"roles": "library_admin,library_author"}, 1), ({"roles": "library_author,library_user"}, 2), ({"roles": "library_user,library_admin"}, 3), ({"roles": "library_admin,library_author,library_user"}, 3), # Search by role and username - ({"search": "admin_1", "roles": "library_admin"}, 1), - ({"search": "regular_1", "roles": "library_user"}, 1), - ({"search": "regular_1", "roles": "library_admin"}, 0), + ({"search": "admin_1", "roles": roles.LIBRARY_ADMIN}, 1), + ({"search": "regular_1", "roles": roles.LIBRARY_USER}, 1), + ({"search": "regular_1", "roles": roles.LIBRARY_ADMIN}, 0), # Search by role and email - ({"search": "admin_1@example.com", "roles": "library_admin"}, 1), - ({"search": "@example.com", "roles": "library_admin"}, 1), - ({"search": "@example.com", "roles": "library_user"}, 2), - ({"search": "regular_1@example.com", "roles": "library_admin"}, 0), + ({"search": "admin_1@example.com", "roles": roles.LIBRARY_ADMIN}, 1), + ({"search": "@example.com", "roles": roles.LIBRARY_ADMIN}, 1), + ({"search": "@example.com", "roles": roles.LIBRARY_USER}, 2), + ({"search": "regular_1@example.com", "roles": roles.LIBRARY_ADMIN}, 0), ) @unpack def test_get_users_by_scope_success(self, query_params: dict, expected_count: int): @@ -434,7 +435,7 @@ def test_add_users_to_role_success(self, users: list[str], expected_completed: i - Returns 207 MULTI-STATUS status - Returns appropriate completed and error counts """ - role = "library_admin" + role = roles.LIBRARY_ADMIN request_data = {"role": role, "scope": "lib:Org1:LIB3", "users": users} with patch.object(api.ContentLibraryData, "exists", return_value=True): @@ -457,7 +458,7 @@ def test_add_users_to_role_success(self, users: list[str], expected_completed: i @unpack def test_add_users_to_role_already_has_role(self, users: list[str], expected_completed: int, expected_errors: int): """Test adding users to a role that already has the role.""" - role = "library_user" + role = roles.LIBRARY_USER scope = "lib:Org2:LIB2" request_data = {"role": role, "scope": scope, "users": users} @@ -472,7 +473,7 @@ def test_add_users_to_role_already_has_role(self, users: list[str], expected_com def test_add_users_to_role_exception_handling(self, mock_assign_role_to_user_in_scope): """Test adding users to a role with exception handling.""" request_data = { - "role": "library_admin", + "role": roles.LIBRARY_ADMIN, "scope": "lib:Org1:LIB1", "users": ["regular_1"], } @@ -492,15 +493,15 @@ def test_add_users_to_role_exception_handling(self, mock_assign_role_to_user_in_ @data( {}, - {"role": "library_admin"}, + {"role": roles.LIBRARY_ADMIN}, {"scope": "lib:Org1:LIB1"}, {"users": ["admin_1"]}, - {"role": "library_admin", "scope": "lib:Org1:LIB1"}, + {"role": roles.LIBRARY_ADMIN, "scope": "lib:Org1:LIB1"}, {"scope": "lib:Org1:LIB1", "users": ["admin_1"]}, - {"users": ["admin_1", "regular_1"], "role": "library_admin"}, - {"role": "library_admin", "scope": "lib:Org1:LIB1", "users": []}, + {"users": ["admin_1", "regular_1"], "role": roles.LIBRARY_ADMIN}, + {"role": roles.LIBRARY_ADMIN, "scope": "lib:Org1:LIB1", "users": []}, {"role": "", "scope": "lib:Org1:LIB1", "users": ["admin_1"]}, - {"role": "library_admin", "scope": "", "users": ["admin_1"]}, + {"role": roles.LIBRARY_ADMIN, "scope": "", "users": ["admin_1"]}, ) def test_add_users_to_role_invalid_data(self, request_data: dict): """Test adding users with invalid request data. @@ -531,7 +532,7 @@ def test_add_users_to_role_permissions(self, username: str, status_code: int): - Returns appropriate status code based on permissions """ request_data = { - "role": "library_admin", + "role": roles.LIBRARY_ADMIN, "scope": "lib:Org3:LIB3", "users": ["regular_2"], } @@ -586,7 +587,7 @@ def test_remove_users_from_role_success(self, users: list[str], expected_complet - Returns appropriate completed and error counts """ query_params = { - "role": "library_user", + "role": roles.LIBRARY_USER, "scope": "lib:Org2:LIB2", "users": ",".join(users), } @@ -602,7 +603,7 @@ def test_remove_users_from_role_success(self, users: list[str], expected_complet def test_remove_users_from_role_exception_handling(self, mock_unassign_role_from_user): """Test removing users from a role with exception handling.""" query_params = { - "role": "library_admin", + "role": roles.LIBRARY_ADMIN, "scope": "lib:Org1:LIB1", "users": "regular_1,regular_2,regular_3", } @@ -631,15 +632,15 @@ def test_remove_users_from_role_exception_handling(self, mock_unassign_role_from @data( {}, - {"role": "library_admin"}, + {"role": roles.LIBRARY_ADMIN}, {"scope": "lib:Org1:LIB1"}, {"users": "admin_1"}, - {"role": "library_admin", "scope": "lib:Org1:LIB1"}, + {"role": roles.LIBRARY_ADMIN, "scope": "lib:Org1:LIB1"}, {"scope": "lib:Org1:LIB1", "users": "admin_1"}, - {"users": "admin_1,regular_1", "role": "library_admin"}, - {"role": "library_admin", "scope": "lib:Org1:LIB1", "users": ""}, + {"users": "admin_1,regular_1", "role": roles.LIBRARY_ADMIN}, + {"role": roles.LIBRARY_ADMIN, "scope": "lib:Org1:LIB1", "users": ""}, {"role": "", "scope": "lib:Org1:LIB1", "users": "admin_1"}, - {"role": "library_admin", "scope": "", "users": "admin_1"}, + {"role": roles.LIBRARY_ADMIN, "scope": "", "users": "admin_1"}, ) def test_remove_users_from_role_invalid_params(self, query_params: dict): """Test removing users with invalid query parameters. @@ -669,7 +670,7 @@ def test_remove_users_from_role_permissions(self, username: str, status_code: in - Returns appropriate status code based on permissions """ query_params = { - "role": "library_admin", + "role": roles.LIBRARY_ADMIN, "scope": "lib:Org3:LIB3", "users": "user1,user2", } diff --git a/openedx_authz/tests/test_commands.py b/openedx_authz/tests/test_commands.py index e12e49fb..7e164adf 100644 --- a/openedx_authz/tests/test_commands.py +++ b/openedx_authz/tests/test_commands.py @@ -13,6 +13,7 @@ from openedx_authz import ROOT_DIRECTORY from openedx_authz import api as authz_api +from openedx_authz.constants import permissions from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.management.commands.load_policies import Command as LoadPoliciesCommand @@ -140,7 +141,7 @@ def test_interactive_mode_allowed_request(self, mock_is_allowed: Mock, mock_get_ output = self.buffer.getvalue() self.assertIn("✓ ALLOWED: alice view_library lib:Org1:LIB1", output) - mock_is_allowed.assert_called_once_with("alice", "view_library", "lib:Org1:LIB1") + mock_is_allowed.assert_called_once_with("alice", permissions.VIEW_LIBRARY, "lib:Org1:LIB1") @patch.object(AuthzEnforcer, "get_enforcer") @patch.object(authz_api, "is_user_allowed") @@ -154,7 +155,7 @@ def test_interactive_mode_denied_request(self, mock_is_allowed: Mock, mock_get_e output = self.buffer.getvalue() self.assertIn("✗ DENIED: bob delete_library lib:Org2:LIB2", output) - mock_is_allowed.assert_called_once_with("bob", "delete_library", "lib:Org2:LIB2") + mock_is_allowed.assert_called_once_with("bob", permissions.DELETE_LIBRARY, "lib:Org2:LIB2") @patch("openedx_authz.management.commands.enforcement.Enforcer") def test_interactive_mode_file_mode_enforcement(self, mock_enforcer_class: Mock): diff --git a/openedx_authz/tests/test_enforcement.py b/openedx_authz/tests/test_enforcement.py index 4938be1d..dbbd65e3 100644 --- a/openedx_authz/tests/test_enforcement.py +++ b/openedx_authz/tests/test_enforcement.py @@ -13,6 +13,7 @@ from ddt import data, ddt, unpack from openedx_authz import ROOT_DIRECTORY +from openedx_authz.constants import roles from openedx_authz.tests.test_utils import ( make_action_key, make_library_key, @@ -257,7 +258,7 @@ class RoleAssignmentTests(CasbinEnforcementTestCase): ], [ "p", - make_role_key("library_admin"), + make_role_key(roles.LIBRARY_ADMIN), make_action_key("manage"), make_scope_key("lib", "*"), "allow", @@ -278,7 +279,7 @@ class RoleAssignmentTests(CasbinEnforcementTestCase): ], [ "p", - make_role_key("library_author"), + make_role_key(roles.LIBRARY_AUTHOR), make_action_key("write"), make_scope_key("lib", "*"), "allow", @@ -312,7 +313,7 @@ class RoleAssignmentTests(CasbinEnforcementTestCase): [ "g", make_user_key("user-6"), - make_role_key("library_admin"), + make_role_key(roles.LIBRARY_ADMIN), make_library_key("lib:DemoX:CSPROB"), ], [ @@ -330,7 +331,7 @@ class RoleAssignmentTests(CasbinEnforcementTestCase): [ "g", make_user_key("user-9"), - make_role_key("library_author"), + make_role_key(roles.LIBRARY_AUTHOR), make_library_key("lib:DemoX:CSPROB"), ], ] + COMMON_ACTION_GROUPING @@ -493,7 +494,7 @@ class WildcardScopeTests(CasbinEnforcementTestCase): ], [ "p", - make_role_key("library_admin"), + make_role_key(roles.LIBRARY_ADMIN), make_action_key("manage"), make_scope_key("lib", "*"), "allow", @@ -502,7 +503,7 @@ class WildcardScopeTests(CasbinEnforcementTestCase): ["g", make_user_key("user-1"), make_role_key("platform_admin"), "*"], ["g", make_user_key("user-2"), make_role_key("org_admin"), "*"], ["g", make_user_key("user-3"), make_role_key("course_admin"), "*"], - ["g", make_user_key("user-4"), make_role_key("library_admin"), "*"], + ["g", make_user_key("user-4"), make_role_key(roles.LIBRARY_ADMIN), "*"], ] + COMMON_ACTION_GROUPING @data( diff --git a/openedx_authz/tests/test_engine_utils.py b/openedx_authz/tests/test_engine_utils.py index 74f33321..7ec3118e 100644 --- a/openedx_authz/tests/test_engine_utils.py +++ b/openedx_authz/tests/test_engine_utils.py @@ -15,6 +15,7 @@ from django.test import TestCase from openedx_authz import ROOT_DIRECTORY +from openedx_authz.constants import permissions, roles from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.engine.utils import migrate_policy_between_enforcers from openedx_authz.tests.test_utils import make_action_key, make_role_key, make_scope_key, make_user_key @@ -92,8 +93,8 @@ def test_migrate_all_file_policies_to_database(self): self.assertIn( [ - make_role_key("library_admin"), - make_action_key("delete_library"), + make_role_key(roles.LIBRARY_ADMIN), + make_action_key(permissions.DELETE_LIBRARY), make_scope_key("lib", "*"), "allow", ], @@ -101,8 +102,8 @@ def test_migrate_all_file_policies_to_database(self): ) self.assertIn( [ - make_role_key("library_user"), - make_action_key("view_library"), + make_role_key(roles.LIBRARY_USER), + make_action_key(permissions.VIEW_LIBRARY), make_scope_key("lib", "*"), "allow", ], @@ -151,15 +152,15 @@ def test_migrate_action_inheritance_from_file(self): # Verify a sample of expected g2 rules from the file self.assertIn( [ - make_action_key("delete_library"), - make_action_key("edit_library_content"), + make_action_key(permissions.DELETE_LIBRARY), + make_action_key(permissions.EDIT_LIBRARY_CONTENT), ], target_g2, ) self.assertIn( [ - make_action_key("manage_library_team"), - make_action_key("view_library_team"), + make_action_key(permissions.MANAGE_LIBRARY_TEAM), + make_action_key(permissions.VIEW_LIBRARY_TEAM), ], target_g2, ) @@ -238,8 +239,8 @@ def test_migrate_partial_duplicates(self): - Mixed state is handled correctly """ self.target_enforcer.add_policy( - make_role_key("library_admin"), - make_action_key("delete_library"), + make_role_key(roles.LIBRARY_ADMIN), + make_action_key(permissions.DELETE_LIBRARY), make_scope_key("lib", "*"), "allow", ) @@ -263,18 +264,18 @@ def test_migrate_partial_duplicates(self): @data( ( - make_role_key("library_admin"), - make_action_key("delete_library"), + make_role_key(roles.LIBRARY_ADMIN), + make_action_key(permissions.DELETE_LIBRARY), make_scope_key("lib", "*"), ), ( - make_role_key("library_user"), - make_action_key("view_library"), + make_role_key(roles.LIBRARY_USER), + make_action_key(permissions.VIEW_LIBRARY), make_scope_key("lib", "*"), ), ( - make_role_key("library_author"), - make_action_key("edit_library_content"), + make_role_key(roles.LIBRARY_AUTHOR), + make_action_key(permissions.EDIT_LIBRARY_CONTENT), make_scope_key("lib", "*"), ), ) @@ -296,9 +297,9 @@ def test_migrate_specific_file_policies(self, role, action, scope): ) @data( - (make_action_key("delete_library"), make_action_key("edit_library_content")), - (make_action_key("edit_library_content"), make_action_key("view_library")), - (make_action_key("manage_library_team"), make_action_key("view_library_team")), + (make_action_key(permissions.DELETE_LIBRARY), make_action_key(permissions.EDIT_LIBRARY_CONTENT)), + (make_action_key(permissions.EDIT_LIBRARY_CONTENT), make_action_key(permissions.VIEW_LIBRARY)), + (make_action_key(permissions.MANAGE_LIBRARY_TEAM), make_action_key(permissions.VIEW_LIBRARY_TEAM)), ) @unpack def test_migrate_specific_action_inheritance(self, parent_action, child_action): @@ -349,12 +350,12 @@ def test_migrate_preserves_user_role_assignments_in_db(self): """ self.target_enforcer.add_grouping_policy( make_user_key("user-1"), - make_role_key("library_admin"), + make_role_key(roles.LIBRARY_ADMIN), make_scope_key("lib", "demo"), ) self.target_enforcer.add_grouping_policy( make_user_key("user-2"), - make_role_key("library_user"), + make_role_key(roles.LIBRARY_USER), make_scope_key("lib", "*"), ) @@ -365,7 +366,7 @@ def test_migrate_preserves_user_role_assignments_in_db(self): self.assertIn( [ make_user_key("user-1"), - make_role_key("library_admin"), + make_role_key(roles.LIBRARY_ADMIN), make_scope_key("lib", "demo"), ], target_grouping, From 1371d5e01d780e3491b8e91f6b47633a778d2489 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Tue, 28 Oct 2025 13:16:02 -0500 Subject: [PATCH 3/4] docs: bumpversion --- CHANGELOG.rst | 8 ++++++++ openedx_authz/__init__.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 26b7d4f6..6f7047e9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,14 @@ Unreleased * +0.10.1 - 2025-10-28 +******************** + +Fixed +===== + +* Fix constants and test class to be able to use it outside this app. + 0.10.0 - 2025-10-28 ******************* diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index bd17868e..fcdedfbc 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "0.10.0" +__version__ = "0.10.1" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__)) From 53965709cc6912dd265ce7785da07f7a9cca2059 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Tue, 28 Oct 2025 17:28:49 -0500 Subject: [PATCH 4/4] fix: apply feedback --- openedx_authz/constants/permissions.py | 57 ++++- openedx_authz/constants/roles.py | 178 ++++----------- openedx_authz/rest_api/v1/views.py | 8 +- openedx_authz/tests/api/test_data.py | 10 +- openedx_authz/tests/api/test_roles.py | 254 ++++++++++----------- openedx_authz/tests/api/test_users.py | 70 +++--- openedx_authz/tests/rest_api/test_views.py | 82 +++---- openedx_authz/tests/test_commands.py | 4 +- openedx_authz/tests/test_enforcement.py | 12 +- openedx_authz/tests/test_engine_utils.py | 53 +++-- 10 files changed, 341 insertions(+), 387 deletions(-) diff --git a/openedx_authz/constants/permissions.py b/openedx_authz/constants/permissions.py index 27268457..f108d6d7 100644 --- a/openedx_authz/constants/permissions.py +++ b/openedx_authz/constants/permissions.py @@ -1,16 +1,51 @@ """ Default permission constants. """ +from openedx_authz.api.data import ActionData, PermissionData # Content Library Permissions -VIEW_LIBRARY = "view_library" -MANAGE_LIBRARY_TAGS = "manage_library_tags" -DELETE_LIBRARY = "delete_library" -EDIT_LIBRARY_CONTENT = "edit_library_content" -PUBLISH_LIBRARY_CONTENT = "publish_library_content" -REUSE_LIBRARY_CONTENT = "reuse_library_content" -VIEW_LIBRARY_TEAM = "view_library_team" -MANAGE_LIBRARY_TEAM = "manage_library_team" -CREATE_LIBRARY_COLLECTION = "create_library_collection" -EDIT_LIBRARY_COLLECTION = "edit_library_collection" -DELETE_LIBRARY_COLLECTION = "delete_library_collection" +VIEW_LIBRARY = PermissionData( + action=ActionData(external_key="view_library"), + effect="allow", +) +MANAGE_LIBRARY_TAGS = PermissionData( + action=ActionData(external_key="manage_library_tags"), + effect="allow", +) +DELETE_LIBRARY = PermissionData( + action=ActionData(external_key="delete_library"), + effect="allow", +) +EDIT_LIBRARY_CONTENT = PermissionData( + action=ActionData(external_key="edit_library_content"), + effect="allow", +) +PUBLISH_LIBRARY_CONTENT = PermissionData( + action=ActionData(external_key="publish_library_content"), + effect="allow", +) +REUSE_LIBRARY_CONTENT = PermissionData( + action=ActionData(external_key="reuse_library_content"), + effect="allow", +) +VIEW_LIBRARY_TEAM = PermissionData( + action=ActionData(external_key="view_library_team"), + effect="allow", +) +MANAGE_LIBRARY_TEAM = PermissionData( + action=ActionData(external_key="manage_library_team"), + effect="allow", +) + +CREATE_LIBRARY_COLLECTION = PermissionData( + action=ActionData(external_key="create_library_collection"), + effect="allow", +) +EDIT_LIBRARY_COLLECTION = PermissionData( + action=ActionData(external_key="edit_library_collection"), + effect="allow", +) +DELETE_LIBRARY_COLLECTION = PermissionData( + action=ActionData(external_key="delete_library_collection"), + effect="allow", +) diff --git a/openedx_authz/constants/roles.py b/openedx_authz/constants/roles.py index 2c320a94..44da76fa 100644 --- a/openedx_authz/constants/roles.py +++ b/openedx_authz/constants/roles.py @@ -2,147 +2,57 @@ Default roles and their associated permissions. """ -from openedx_authz.api.data import ActionData, PermissionData +from openedx_authz.api.data import RoleData from openedx_authz.constants import permissions -# Library Roles -LIBRARY_ADMIN = "library_admin" -LIBRARY_AUTHOR = "library_author" -LIBRARY_CONTRIBUTOR = "library_contributor" -LIBRARY_USER = "library_user" +# Library Roles and Permissions -LIST_LIBRARY_ADMIN_PERMISSIONS = [ - PermissionData( - action=ActionData(external_key=permissions.VIEW_LIBRARY), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.MANAGE_LIBRARY_TAGS), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.DELETE_LIBRARY), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.EDIT_LIBRARY_CONTENT), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.PUBLISH_LIBRARY_CONTENT), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.REUSE_LIBRARY_CONTENT), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.VIEW_LIBRARY_TEAM), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.MANAGE_LIBRARY_TEAM), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.CREATE_LIBRARY_COLLECTION), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.EDIT_LIBRARY_COLLECTION), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.DELETE_LIBRARY_COLLECTION), - effect="allow", - ), +# Define the associated permissions for each role + +LIBRARY_ADMIN_PERMISSIONS = [ + permissions.VIEW_LIBRARY, + permissions.MANAGE_LIBRARY_TAGS, + permissions.DELETE_LIBRARY, + permissions.EDIT_LIBRARY_CONTENT, + permissions.PUBLISH_LIBRARY_CONTENT, + permissions.REUSE_LIBRARY_CONTENT, + permissions.VIEW_LIBRARY_TEAM, + permissions.MANAGE_LIBRARY_TEAM, + permissions.CREATE_LIBRARY_COLLECTION, + permissions.EDIT_LIBRARY_COLLECTION, + permissions.DELETE_LIBRARY_COLLECTION, ] -LIST_LIBRARY_AUTHOR_PERMISSIONS = [ - PermissionData( - action=ActionData(external_key=permissions.VIEW_LIBRARY), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.MANAGE_LIBRARY_TAGS), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.EDIT_LIBRARY_CONTENT), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.PUBLISH_LIBRARY_CONTENT), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.REUSE_LIBRARY_CONTENT), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.VIEW_LIBRARY_TEAM), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.CREATE_LIBRARY_COLLECTION), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.EDIT_LIBRARY_COLLECTION), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.DELETE_LIBRARY_COLLECTION), - effect="allow", - ), +LIBRARY_AUTHOR_PERMISSIONS = [ + permissions.VIEW_LIBRARY, + permissions.MANAGE_LIBRARY_TAGS, + permissions.EDIT_LIBRARY_CONTENT, + permissions.PUBLISH_LIBRARY_CONTENT, + permissions.REUSE_LIBRARY_CONTENT, + permissions.VIEW_LIBRARY_TEAM, + permissions.CREATE_LIBRARY_COLLECTION, + permissions.EDIT_LIBRARY_COLLECTION, + permissions.DELETE_LIBRARY_COLLECTION, ] -LIST_LIBRARY_CONTRIBUTOR_PERMISSIONS = [ - PermissionData( - action=ActionData(external_key=permissions.VIEW_LIBRARY), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.MANAGE_LIBRARY_TAGS), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.EDIT_LIBRARY_CONTENT), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.REUSE_LIBRARY_CONTENT), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.VIEW_LIBRARY_TEAM), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.CREATE_LIBRARY_COLLECTION), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.EDIT_LIBRARY_COLLECTION), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.DELETE_LIBRARY_COLLECTION), - effect="allow", - ), +LIBRARY_CONTRIBUTOR_PERMISSIONS = [ + permissions.VIEW_LIBRARY, + permissions.MANAGE_LIBRARY_TAGS, + permissions.EDIT_LIBRARY_CONTENT, + permissions.REUSE_LIBRARY_CONTENT, + permissions.VIEW_LIBRARY_TEAM, + permissions.CREATE_LIBRARY_COLLECTION, + permissions.EDIT_LIBRARY_COLLECTION, + permissions.DELETE_LIBRARY_COLLECTION, ] -LIST_LIBRARY_USER_PERMISSIONS = [ - PermissionData( - action=ActionData(external_key=permissions.VIEW_LIBRARY), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.REUSE_LIBRARY_CONTENT), - effect="allow", - ), - PermissionData( - action=ActionData(external_key=permissions.VIEW_LIBRARY_TEAM), - effect="allow", - ), +LIBRARY_USER_PERMISSIONS = [ + permissions.VIEW_LIBRARY, + permissions.REUSE_LIBRARY_CONTENT, + permissions.VIEW_LIBRARY_TEAM, ] + +LIBRARY_ADMIN = RoleData(external_key="library_admin", permissions=LIBRARY_ADMIN_PERMISSIONS) +LIBRARY_AUTHOR = RoleData(external_key="library_author", permissions=LIBRARY_AUTHOR_PERMISSIONS) +LIBRARY_CONTRIBUTOR = RoleData(external_key="library_contributor", permissions=LIBRARY_CONTRIBUTOR_PERMISSIONS) +LIBRARY_USER = RoleData(external_key="library_user", permissions=LIBRARY_USER_PERMISSIONS) diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index bb16789b..a5b9376d 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -251,7 +251,7 @@ class RoleUserAPIView(APIView): status.HTTP_401_UNAUTHORIZED: "The user is not authenticated or does not have the required permissions", }, ) - @authz_permissions([permissions.VIEW_LIBRARY_TEAM]) + @authz_permissions([permissions.VIEW_LIBRARY.identifier]) def get(self, request: HttpRequest) -> Response: """Retrieve all users with role assignments within a specific scope.""" serializer = ListUsersInRoleWithScopeSerializer(data=request.query_params) @@ -278,7 +278,7 @@ def get(self, request: HttpRequest) -> Response: status.HTTP_401_UNAUTHORIZED: "The user is not authenticated or does not have the required permissions", }, ) - @authz_permissions([permissions.MANAGE_LIBRARY_TEAM]) + @authz_permissions([permissions.MANAGE_LIBRARY_TEAM.identifier]) def put(self, request: HttpRequest) -> Response: """Assign multiple users to a specific role within a scope.""" serializer = AddUsersToRoleWithScopeSerializer(data=request.data) @@ -325,7 +325,7 @@ def put(self, request: HttpRequest) -> Response: status.HTTP_401_UNAUTHORIZED: "The user is not authenticated or does not have the required permissions", }, ) - @authz_permissions([permissions.MANAGE_LIBRARY_TEAM]) + @authz_permissions([permissions.MANAGE_LIBRARY_TEAM.identifier]) def delete(self, request: HttpRequest) -> Response: """Remove multiple users from a specific role within a scope.""" serializer = RemoveUsersFromRoleWithScopeSerializer(data=request.query_params) @@ -428,7 +428,7 @@ class RoleListView(APIView): status.HTTP_401_UNAUTHORIZED: "The user is not authenticated or does not have the required permissions", }, ) - @authz_permissions([permissions.VIEW_LIBRARY_TEAM]) + @authz_permissions([permissions.VIEW_LIBRARY.identifier]) def get(self, request: HttpRequest) -> Response: """Retrieve all roles and their permissions for a specific scope.""" serializer = ListRolesWithScopeSerializer(data=request.query_params) diff --git a/openedx_authz/tests/api/test_data.py b/openedx_authz/tests/api/test_data.py index fb087105..614d6cb3 100644 --- a/openedx_authz/tests/api/test_data.py +++ b/openedx_authz/tests/api/test_data.py @@ -373,7 +373,7 @@ def test_user_data_str_and_repr(self, external_key, expected_str, expected_repr) @data( ("read", "Read", "act^read"), ("write", "Write", "act^write"), - (permissions.DELETE_LIBRARY, "Delete Library", "act^delete_library"), + (permissions.DELETE_LIBRARY.identifier, "Delete Library", "act^delete_library"), ("edit_content", "Edit Content", "act^edit_content"), ) @unpack @@ -414,7 +414,7 @@ def test_scope_data_str_and_repr(self, external_key, expected_str, expected_repr @data( ("instructor", "Instructor", "role^instructor"), - (roles.LIBRARY_ADMIN, "Library Admin", "role^library_admin"), + (roles.LIBRARY_ADMIN.external_key, "Library Admin", "role^library_admin"), ("course_staff", "Course Staff", "role^course_staff"), ) @unpack @@ -455,7 +455,7 @@ def test_role_data_str_with_permissions(self): ("read", "allow", "Read - allow", "act^read => allow"), ("write", "deny", "Write - deny", "act^write => deny"), ( - permissions.DELETE_LIBRARY, + permissions.DELETE_LIBRARY.identifier, "allow", "Delete Library - allow", "act^delete_library => allow", @@ -486,7 +486,7 @@ def test_role_assignment_data_str(self): """ user = UserData(external_key="john_doe") role1 = RoleData(external_key="instructor") - role2 = RoleData(external_key=roles.LIBRARY_ADMIN) + role2 = RoleData(external_key=roles.LIBRARY_ADMIN.external_key) scope = ContentLibraryData(external_key="lib:DemoX:CSPROB") assignment = RoleAssignmentData(subject=user, roles=[role1, role2], scope=scope) @@ -503,7 +503,7 @@ def test_role_assignment_data_repr(self): """ user = UserData(external_key="john_doe") role1 = RoleData(external_key="instructor") - role2 = RoleData(external_key=roles.LIBRARY_ADMIN) + role2 = RoleData(external_key=roles.LIBRARY_ADMIN.external_key) scope = ContentLibraryData(external_key="lib:DemoX:CSPROB") assignment = RoleAssignmentData(subject=user, roles=[role1, role2], scope=scope) diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index 089cea4a..4676aba0 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -28,10 +28,10 @@ ) from openedx_authz.constants import roles from openedx_authz.constants.roles import ( - LIST_LIBRARY_ADMIN_PERMISSIONS, - LIST_LIBRARY_AUTHOR_PERMISSIONS, - LIST_LIBRARY_CONTRIBUTOR_PERMISSIONS, - LIST_LIBRARY_USER_PERMISSIONS, + LIBRARY_ADMIN_PERMISSIONS, + LIBRARY_AUTHOR_PERMISSIONS, + LIBRARY_CONTRIBUTOR_PERMISSIONS, + LIBRARY_USER_PERMISSIONS, ) from openedx_authz.engine.enforcer import AuthzEnforcer from openedx_authz.engine.utils import migrate_policy_between_enforcers @@ -126,123 +126,123 @@ def setUpClass(cls): # Basic library roles from authz.policy { "subject_name": "alice", - "role_name": roles.LIBRARY_ADMIN, + "role_name": roles.LIBRARY_ADMIN.external_key, "scope_name": "lib:Org1:math_101", }, { "subject_name": "bob", - "role_name": roles.LIBRARY_AUTHOR, + "role_name": roles.LIBRARY_AUTHOR.external_key, "scope_name": "lib:Org1:history_201", }, { "subject_name": "carol", - "role_name": roles.LIBRARY_CONTRIBUTOR, + "role_name": roles.LIBRARY_CONTRIBUTOR.external_key, "scope_name": "lib:Org1:science_301", }, { "subject_name": "dave", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org1:english_101", }, # Multi-role assignments - same subject with different roles in different libraries { "subject_name": "eve", - "role_name": roles.LIBRARY_ADMIN, + "role_name": roles.LIBRARY_ADMIN.external_key, "scope_name": "lib:Org2:physics_401", }, { "subject_name": "eve", - "role_name": roles.LIBRARY_AUTHOR, + "role_name": roles.LIBRARY_AUTHOR.external_key, "scope_name": "lib:Org2:chemistry_501", }, { "subject_name": "eve", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org2:biology_601", }, # Multiple subjects with same role in same scope { "subject_name": "grace", - "role_name": roles.LIBRARY_CONTRIBUTOR, + "role_name": roles.LIBRARY_CONTRIBUTOR.external_key, "scope_name": "lib:Org1:math_advanced", }, { "subject_name": "heidi", - "role_name": roles.LIBRARY_CONTRIBUTOR, + "role_name": roles.LIBRARY_CONTRIBUTOR.external_key, "scope_name": "lib:Org1:math_advanced", }, # Hierarchical scope assignments - different specificity levels { "subject_name": "ivy", - "role_name": roles.LIBRARY_ADMIN, + "role_name": roles.LIBRARY_ADMIN.external_key, "scope_name": "lib:Org3:cs_101", }, { "subject_name": "jack", - "role_name": roles.LIBRARY_AUTHOR, + "role_name": roles.LIBRARY_AUTHOR.external_key, "scope_name": "lib:Org3:cs_101", }, { "subject_name": "kate", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org3:cs_101", }, # Edge case: same user, same role, different scopes { "subject_name": "liam", - "role_name": roles.LIBRARY_AUTHOR, + "role_name": roles.LIBRARY_AUTHOR.external_key, "scope_name": "lib:Org4:art_101", }, { "subject_name": "liam", - "role_name": roles.LIBRARY_AUTHOR, + "role_name": roles.LIBRARY_AUTHOR.external_key, "scope_name": "lib:Org4:art_201", }, { "subject_name": "liam", - "role_name": roles.LIBRARY_AUTHOR, + "role_name": roles.LIBRARY_AUTHOR.external_key, "scope_name": "lib:Org4:art_301", }, # Mixed permission levels across libraries for comprehensive testing { "subject_name": "maya", - "role_name": roles.LIBRARY_ADMIN, + "role_name": roles.LIBRARY_ADMIN.external_key, "scope_name": "lib:Org5:economics_101", }, { "subject_name": "noah", - "role_name": roles.LIBRARY_CONTRIBUTOR, + "role_name": roles.LIBRARY_CONTRIBUTOR.external_key, "scope_name": "lib:Org5:economics_101", }, { "subject_name": "olivia", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org5:economics_101", }, # Complex multi-library, multi-role scenario { "subject_name": "peter", - "role_name": roles.LIBRARY_ADMIN, + "role_name": roles.LIBRARY_ADMIN.external_key, "scope_name": "lib:Org6:project_alpha", }, { "subject_name": "peter", - "role_name": roles.LIBRARY_AUTHOR, + "role_name": roles.LIBRARY_AUTHOR.external_key, "scope_name": "lib:Org6:project_beta", }, { "subject_name": "peter", - "role_name": roles.LIBRARY_CONTRIBUTOR, + "role_name": roles.LIBRARY_CONTRIBUTOR.external_key, "scope_name": "lib:Org6:project_gamma", }, { "subject_name": "peter", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org6:project_delta", }, { "subject_name": "frank", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org6:project_epsilon", }, ] @@ -272,23 +272,23 @@ class TestRolesAPI(RolesTestSetupMixin): @ddt_data( # Library Admin role with actual permissions from authz.policy ( - roles.LIBRARY_ADMIN, - LIST_LIBRARY_ADMIN_PERMISSIONS, + roles.LIBRARY_ADMIN.external_key, + LIBRARY_ADMIN_PERMISSIONS, ), # Library Author role with actual permissions from authz.policy ( - roles.LIBRARY_AUTHOR, - LIST_LIBRARY_AUTHOR_PERMISSIONS, + roles.LIBRARY_AUTHOR.external_key, + LIBRARY_AUTHOR_PERMISSIONS, ), # Library Contributor role with actual permissions from authz.policy ( - roles.LIBRARY_CONTRIBUTOR, - LIST_LIBRARY_CONTRIBUTOR_PERMISSIONS, + roles.LIBRARY_CONTRIBUTOR.external_key, + LIBRARY_CONTRIBUTOR_PERMISSIONS, ), # Library User role with minimal permissions ( - roles.LIBRARY_USER, - LIST_LIBRARY_USER_PERMISSIONS, + roles.LIBRARY_USER.external_key, + LIBRARY_USER_PERMISSIONS, ), # Non existent role ( @@ -311,21 +311,21 @@ def test_get_permissions_for_roles(self, role_name, expected_permissions): @ddt_data( # Role assigned to multiple users in different scopes ( - roles.LIBRARY_USER, + roles.LIBRARY_USER.external_key, "lib:Org1:english_101", - LIST_LIBRARY_USER_PERMISSIONS, + LIBRARY_USER_PERMISSIONS, ), # Role assigned to single user in single scope ( - roles.LIBRARY_AUTHOR, + roles.LIBRARY_AUTHOR.external_key, "lib:Org1:history_201", - LIST_LIBRARY_AUTHOR_PERMISSIONS, + LIBRARY_AUTHOR_PERMISSIONS, ), # Role assigned to single user in multiple scopes ( - roles.LIBRARY_ADMIN, + roles.LIBRARY_ADMIN.external_key, "lib:Org1:math_101", - LIST_LIBRARY_ADMIN_PERMISSIONS, + LIBRARY_ADMIN_PERMISSIONS, ), ) @unpack @@ -350,10 +350,10 @@ def test_get_permissions_for_active_role_in_specific_scope(self, role_name, scop ( "*", { - roles.LIBRARY_ADMIN, - roles.LIBRARY_AUTHOR, - roles.LIBRARY_CONTRIBUTOR, - roles.LIBRARY_USER, + roles.LIBRARY_ADMIN.external_key, + roles.LIBRARY_AUTHOR.external_key, + roles.LIBRARY_CONTRIBUTOR.external_key, + roles.LIBRARY_USER.external_key, }, ), ) @@ -377,27 +377,27 @@ def test_get_roles_in_scope(self, scope_name, expected_roles): self.assertEqual(role_names, expected_roles) @ddt_data( - ("alice", "lib:Org1:math_101", {roles.LIBRARY_ADMIN}), - ("bob", "lib:Org1:history_201", {roles.LIBRARY_AUTHOR}), - ("carol", "lib:Org1:science_301", {roles.LIBRARY_CONTRIBUTOR}), - ("dave", "lib:Org1:english_101", {roles.LIBRARY_USER}), - ("eve", "lib:Org2:physics_401", {roles.LIBRARY_ADMIN}), - ("eve", "lib:Org2:chemistry_501", {roles.LIBRARY_AUTHOR}), - ("eve", "lib:Org2:biology_601", {roles.LIBRARY_USER}), - ("grace", "lib:Org1:math_advanced", {roles.LIBRARY_CONTRIBUTOR}), - ("ivy", "lib:Org3:cs_101", {roles.LIBRARY_ADMIN}), - ("jack", "lib:Org3:cs_101", {roles.LIBRARY_AUTHOR}), - ("kate", "lib:Org3:cs_101", {roles.LIBRARY_USER}), - ("liam", "lib:Org4:art_101", {roles.LIBRARY_AUTHOR}), - ("liam", "lib:Org4:art_201", {roles.LIBRARY_AUTHOR}), - ("liam", "lib:Org4:art_301", {roles.LIBRARY_AUTHOR}), - ("maya", "lib:Org5:economics_101", {roles.LIBRARY_ADMIN}), - ("noah", "lib:Org5:economics_101", {roles.LIBRARY_CONTRIBUTOR}), - ("olivia", "lib:Org5:economics_101", {roles.LIBRARY_USER}), - ("peter", "lib:Org6:project_alpha", {roles.LIBRARY_ADMIN}), - ("peter", "lib:Org6:project_beta", {roles.LIBRARY_AUTHOR}), - ("peter", "lib:Org6:project_gamma", {roles.LIBRARY_CONTRIBUTOR}), - ("peter", "lib:Org6:project_delta", {roles.LIBRARY_USER}), + ("alice", "lib:Org1:math_101", {roles.LIBRARY_ADMIN.external_key}), + ("bob", "lib:Org1:history_201", {roles.LIBRARY_AUTHOR.external_key}), + ("carol", "lib:Org1:science_301", {roles.LIBRARY_CONTRIBUTOR.external_key}), + ("dave", "lib:Org1:english_101", {roles.LIBRARY_USER.external_key}), + ("eve", "lib:Org2:physics_401", {roles.LIBRARY_ADMIN.external_key}), + ("eve", "lib:Org2:chemistry_501", {roles.LIBRARY_AUTHOR.external_key}), + ("eve", "lib:Org2:biology_601", {roles.LIBRARY_USER.external_key}), + ("grace", "lib:Org1:math_advanced", {roles.LIBRARY_CONTRIBUTOR.external_key}), + ("ivy", "lib:Org3:cs_101", {roles.LIBRARY_ADMIN.external_key}), + ("jack", "lib:Org3:cs_101", {roles.LIBRARY_AUTHOR.external_key}), + ("kate", "lib:Org3:cs_101", {roles.LIBRARY_USER.external_key}), + ("liam", "lib:Org4:art_101", {roles.LIBRARY_AUTHOR.external_key}), + ("liam", "lib:Org4:art_201", {roles.LIBRARY_AUTHOR.external_key}), + ("liam", "lib:Org4:art_301", {roles.LIBRARY_AUTHOR.external_key}), + ("maya", "lib:Org5:economics_101", {roles.LIBRARY_ADMIN.external_key}), + ("noah", "lib:Org5:economics_101", {roles.LIBRARY_CONTRIBUTOR.external_key}), + ("olivia", "lib:Org5:economics_101", {roles.LIBRARY_USER.external_key}), + ("peter", "lib:Org6:project_alpha", {roles.LIBRARY_ADMIN.external_key}), + ("peter", "lib:Org6:project_beta", {roles.LIBRARY_AUTHOR.external_key}), + ("peter", "lib:Org6:project_gamma", {roles.LIBRARY_CONTRIBUTOR.external_key}), + ("peter", "lib:Org6:project_delta", {roles.LIBRARY_USER.external_key}), ("non_existent_user", "lib:Org1:math_101", set()), ("alice", "lib:Org999:non_existent_scope", set()), ("non_existent_user", "lib:Org999:non_existent_scope", set()), @@ -421,8 +421,8 @@ def test_get_subject_role_assignments_in_scope(self, subject_name, scope_name, e "alice", [ RoleData( - external_key=roles.LIBRARY_ADMIN, - permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, + external_key=roles.LIBRARY_ADMIN.external_key, + permissions=LIBRARY_ADMIN_PERMISSIONS, ), ], ), @@ -430,16 +430,16 @@ def test_get_subject_role_assignments_in_scope(self, subject_name, scope_name, e "eve", [ RoleData( - external_key=roles.LIBRARY_ADMIN, - permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, + external_key=roles.LIBRARY_ADMIN.external_key, + permissions=LIBRARY_ADMIN_PERMISSIONS, ), RoleData( - external_key=roles.LIBRARY_AUTHOR, - permissions=LIST_LIBRARY_AUTHOR_PERMISSIONS, + external_key=roles.LIBRARY_AUTHOR.external_key, + permissions=LIBRARY_AUTHOR_PERMISSIONS, ), RoleData( - external_key=roles.LIBRARY_USER, - permissions=LIST_LIBRARY_USER_PERMISSIONS, + external_key=roles.LIBRARY_USER.external_key, + permissions=LIBRARY_USER_PERMISSIONS, ), ], ), @@ -447,8 +447,8 @@ def test_get_subject_role_assignments_in_scope(self, subject_name, scope_name, e "frank", [ RoleData( - external_key=roles.LIBRARY_USER, - permissions=LIST_LIBRARY_USER_PERMISSIONS, + external_key=roles.LIBRARY_USER.external_key, + permissions=LIBRARY_USER_PERMISSIONS, ), ], ), @@ -471,29 +471,29 @@ def test_get_all_role_assignments_scopes(self, subject_name, expected_roles): self.assertTrue(found, f"Expected role {expected_role} not found in assignments") @ddt_data( - (roles.LIBRARY_ADMIN, "lib:Org1:math_101", 1), - (roles.LIBRARY_AUTHOR, "lib:Org1:history_201", 1), - (roles.LIBRARY_CONTRIBUTOR, "lib:Org1:science_301", 1), - (roles.LIBRARY_USER, "lib:Org1:english_101", 1), - (roles.LIBRARY_ADMIN, "lib:Org2:physics_401", 1), - (roles.LIBRARY_AUTHOR, "lib:Org2:chemistry_501", 1), - (roles.LIBRARY_USER, "lib:Org2:biology_601", 1), - (roles.LIBRARY_CONTRIBUTOR, "lib:Org1:math_advanced", 2), - (roles.LIBRARY_ADMIN, "lib:Org3:cs_101", 1), - (roles.LIBRARY_AUTHOR, "lib:Org3:cs_101", 1), - (roles.LIBRARY_USER, "lib:Org3:cs_101", 1), - (roles.LIBRARY_AUTHOR, "lib:Org4:art_101", 1), - (roles.LIBRARY_AUTHOR, "lib:Org4:art_201", 1), - (roles.LIBRARY_AUTHOR, "lib:Org4:art_301", 1), - (roles.LIBRARY_ADMIN, "lib:Org5:economics_101", 1), - (roles.LIBRARY_CONTRIBUTOR, "lib:Org5:economics_101", 1), - (roles.LIBRARY_USER, "lib:Org5:economics_101", 1), - (roles.LIBRARY_ADMIN, "lib:Org6:project_alpha", 1), - (roles.LIBRARY_AUTHOR, "lib:Org6:project_beta", 1), - (roles.LIBRARY_CONTRIBUTOR, "lib:Org6:project_gamma", 1), - (roles.LIBRARY_USER, "lib:Org6:project_delta", 1), + (roles.LIBRARY_ADMIN.external_key, "lib:Org1:math_101", 1), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org1:history_201", 1), + (roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org1:science_301", 1), + (roles.LIBRARY_USER.external_key, "lib:Org1:english_101", 1), + (roles.LIBRARY_ADMIN.external_key, "lib:Org2:physics_401", 1), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org2:chemistry_501", 1), + (roles.LIBRARY_USER.external_key, "lib:Org2:biology_601", 1), + (roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org1:math_advanced", 2), + (roles.LIBRARY_ADMIN.external_key, "lib:Org3:cs_101", 1), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org3:cs_101", 1), + (roles.LIBRARY_USER.external_key, "lib:Org3:cs_101", 1), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_101", 1), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_201", 1), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_301", 1), + (roles.LIBRARY_ADMIN.external_key, "lib:Org5:economics_101", 1), + (roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org5:economics_101", 1), + (roles.LIBRARY_USER.external_key, "lib:Org5:economics_101", 1), + (roles.LIBRARY_ADMIN.external_key, "lib:Org6:project_alpha", 1), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org6:project_beta", 1), + (roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org6:project_gamma", 1), + (roles.LIBRARY_USER.external_key, "lib:Org6:project_delta", 1), ("non_existent_role", "sc:any_library", 0), - (roles.LIBRARY_ADMIN, "sc:non_existent_scope", 0), + (roles.LIBRARY_ADMIN.external_key, "sc:non_existent_scope", 0), ("non_existent_role", "sc:non_existent_scope", 0), ) @unpack @@ -515,7 +515,7 @@ def test_get_scopes_for_role_and_subject(self): Expected result: - The scopes associated with the specified role and subject are correctly retrieved. """ - role_name = roles.LIBRARY_AUTHOR + role_name = roles.LIBRARY_AUTHOR.external_key subject_name = "liam" expected_scopes = {"lib:Org4:art_101", "lib:Org4:art_201", "lib:Org4:art_301"} @@ -528,11 +528,11 @@ def test_get_scopes_for_role_and_subject(self): self.assertEqual(scope_names, expected_scopes) @ddt_data( - (roles.LIBRARY_AUTHOR, "lib:Org4:art_101", {"liam"}), - (roles.LIBRARY_AUTHOR, "lib:Org4:art_201", {"liam"}), - (roles.LIBRARY_AUTHOR, "lib:Org4:art_301", {"liam"}), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_101", {"liam"}), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_201", {"liam"}), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_301", {"liam"}), ("non_existent_role", "lib:Org4:art_101", set()), - (roles.LIBRARY_AUTHOR, "sc:non_existent_scope", set()), + (roles.LIBRARY_AUTHOR.external_key, "sc:non_existent_scope", set()), ("non_existent_role", "sc:non_existent_scope", set()), ) @unpack @@ -561,24 +561,24 @@ class TestRoleAssignmentAPI(RolesTestSetupMixin): """ @ddt_data( - (["mary", "john"], roles.LIBRARY_USER, "sc:batch_test", True), + (["mary", "john"], roles.LIBRARY_USER.external_key, "sc:batch_test", True), ( ["paul", "diana", "lila"], - roles.LIBRARY_CONTRIBUTOR, + roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org1:math_advanced", True, ), - (["sarina", "ty"], roles.LIBRARY_AUTHOR, "lib:Org4:art_101", True), - (["fran", "bob"], roles.LIBRARY_ADMIN, "lib:Org3:cs_101", True), + (["sarina", "ty"], roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_101", True), + (["fran", "bob"], roles.LIBRARY_ADMIN.external_key, "lib:Org3:cs_101", True), ( ["anna", "tom", "jerry"], - roles.LIBRARY_USER, + roles.LIBRARY_USER.external_key, "lib:Org1:history_201", True, ), - ("joe", roles.LIBRARY_CONTRIBUTOR, "lib:Org1:science_301", False), - ("nina", roles.LIBRARY_AUTHOR, "lib:Org1:english_101", False), - ("oliver", roles.LIBRARY_ADMIN, "lib:Org1:math_101", False), + ("joe", roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org1:science_301", False), + ("nina", roles.LIBRARY_AUTHOR.external_key, "lib:Org1:english_101", False), + ("oliver", roles.LIBRARY_ADMIN.external_key, "lib:Org1:math_101", False), ) @unpack def test_batch_assign_role_to_subjects_in_scope(self, subject_names, role, scope_name, batch): @@ -619,24 +619,24 @@ def test_batch_assign_role_to_subjects_in_scope(self, subject_names, role, scope self.assertIn(role, role_names) @ddt_data( - (["mary", "john"], roles.LIBRARY_USER, "sc:batch_test", True), + (["mary", "john"], roles.LIBRARY_USER.external_key, "sc:batch_test", True), ( ["paul", "diana", "lila"], - roles.LIBRARY_CONTRIBUTOR, + roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org1:math_advanced", True, ), - (["sarina", "ty"], roles.LIBRARY_AUTHOR, "lib:Org4:art_101", True), - (["fran", "bob"], roles.LIBRARY_ADMIN, "lib:Org3:cs_101", True), + (["sarina", "ty"], roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_101", True), + (["fran", "bob"], roles.LIBRARY_ADMIN.external_key, "lib:Org3:cs_101", True), ( ["anna", "tom", "jerry"], - roles.LIBRARY_USER, + roles.LIBRARY_USER.external_key, "lib:Org1:history_201", True, ), - ("joe", roles.LIBRARY_CONTRIBUTOR, "lib:Org1:science_301", False), - ("nina", roles.LIBRARY_AUTHOR, "lib:Org1:english_101", False), - ("oliver", roles.LIBRARY_ADMIN, "lib:Org1:math_101", False), + ("joe", roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org1:science_301", False), + ("nina", roles.LIBRARY_AUTHOR.external_key, "lib:Org1:english_101", False), + ("oliver", roles.LIBRARY_ADMIN.external_key, "lib:Org1:math_101", False), ) @unpack def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_name, batch): @@ -681,8 +681,8 @@ def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_na subject=SubjectData(external_key="alice"), roles=[ RoleData( - external_key=roles.LIBRARY_ADMIN, - permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, + external_key=roles.LIBRARY_ADMIN.external_key, + permissions=LIBRARY_ADMIN_PERMISSIONS, ) ], scope=ScopeData(external_key="lib:Org1:math_101"), @@ -696,8 +696,8 @@ def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_na subject=SubjectData(external_key="bob"), roles=[ RoleData( - external_key=roles.LIBRARY_AUTHOR, - permissions=LIST_LIBRARY_AUTHOR_PERMISSIONS, + external_key=roles.LIBRARY_AUTHOR.external_key, + permissions=LIBRARY_AUTHOR_PERMISSIONS, ) ], scope=ScopeData(external_key="lib:Org1:history_201"), @@ -711,8 +711,8 @@ def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_na subject=SubjectData(external_key="carol"), roles=[ RoleData( - external_key=roles.LIBRARY_CONTRIBUTOR, - permissions=LIST_LIBRARY_CONTRIBUTOR_PERMISSIONS, + external_key=roles.LIBRARY_CONTRIBUTOR.external_key, + permissions=LIBRARY_CONTRIBUTOR_PERMISSIONS, ) ], scope=ScopeData(external_key="lib:Org1:science_301"), @@ -726,8 +726,8 @@ def test_unassign_role_from_subject_in_scope(self, subject_names, role, scope_na subject=SubjectData(external_key="dave"), roles=[ RoleData( - external_key=roles.LIBRARY_USER, - permissions=LIST_LIBRARY_USER_PERMISSIONS, + external_key=roles.LIBRARY_USER.external_key, + permissions=LIBRARY_USER_PERMISSIONS, ) ], scope=ScopeData(external_key="lib:Org1:english_101"), diff --git a/openedx_authz/tests/api/test_users.py b/openedx_authz/tests/api/test_users.py index 23cb2757..369d740b 100644 --- a/openedx_authz/tests/api/test_users.py +++ b/openedx_authz/tests/api/test_users.py @@ -15,7 +15,7 @@ unassign_role_from_user, ) from openedx_authz.constants import permissions, roles -from openedx_authz.constants.roles import LIST_LIBRARY_ADMIN_PERMISSIONS, LIST_LIBRARY_AUTHOR_PERMISSIONS +from openedx_authz.constants.roles import LIBRARY_ADMIN_PERMISSIONS, LIBRARY_AUTHOR_PERMISSIONS from openedx_authz.tests.api.test_roles import RolesTestSetupMixin @@ -52,10 +52,10 @@ class TestUserRoleAssignments(UserAssignmentsSetupMixin): """Test suite for user-role assignment API functions.""" @data( - ("john", roles.LIBRARY_ADMIN, "lib:Org1:math_101", False), - ("jane", roles.LIBRARY_USER, "lib:Org1:english_101", False), - (["mary", "charlie"], roles.LIBRARY_CONTRIBUTOR, "lib:Org1:science_301", True), - (["david", "sarah"], roles.LIBRARY_AUTHOR, "lib:Org1:history_201", True), + ("john", roles.LIBRARY_ADMIN.external_key, "lib:Org1:math_101", False), + ("jane", roles.LIBRARY_USER.external_key, "lib:Org1:english_101", False), + (["mary", "charlie"], roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org1:science_301", True), + (["david", "sarah"], roles.LIBRARY_AUTHOR.external_key, "lib:Org1:history_201", True), ) @unpack def test_assign_role_to_user_in_scope(self, username, role, scope_name, batch): @@ -81,10 +81,10 @@ def test_assign_role_to_user_in_scope(self, username, role, scope_name, batch): self.assertIn(role, role_names) @data( - (["grace"], roles.LIBRARY_CONTRIBUTOR, "lib:Org1:math_advanced", True), - (["liam", "maya"], roles.LIBRARY_AUTHOR, "lib:Org4:art_101", True), - ("alice", roles.LIBRARY_ADMIN, "lib:Org1:math_101", False), - ("bob", roles.LIBRARY_AUTHOR, "lib:Org1:history_201", False), + (["grace"], roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org1:math_advanced", True), + (["liam", "maya"], roles.LIBRARY_AUTHOR.external_key, "lib:Org4:art_101", True), + ("alice", roles.LIBRARY_ADMIN.external_key, "lib:Org1:math_101", False), + ("bob", roles.LIBRARY_AUTHOR.external_key, "lib:Org1:history_201", False), ) @unpack def test_unassign_role_from_user(self, username, role, scope_name, batch): @@ -111,9 +111,9 @@ def test_unassign_role_from_user(self, username, role, scope_name, batch): self.assertNotIn(role, role_names) @data( - ("eve", {roles.LIBRARY_ADMIN, roles.LIBRARY_AUTHOR, roles.LIBRARY_USER}), - ("alice", {roles.LIBRARY_ADMIN}), - ("liam", {roles.LIBRARY_AUTHOR}), + ("eve", {roles.LIBRARY_ADMIN.external_key, roles.LIBRARY_AUTHOR.external_key, roles.LIBRARY_USER.external_key}), + ("alice", {roles.LIBRARY_ADMIN.external_key}), + ("liam", {roles.LIBRARY_AUTHOR.external_key}), ) @unpack def test_get_user_role_assignments(self, username, expected_roles): @@ -129,10 +129,10 @@ def test_get_user_role_assignments(self, username, expected_roles): self.assertEqual(assigned_role_names, expected_roles) @data( - ("alice", "lib:Org1:math_101", {roles.LIBRARY_ADMIN}), - ("bob", "lib:Org1:history_201", {roles.LIBRARY_AUTHOR}), - ("eve", "lib:Org2:physics_401", {roles.LIBRARY_ADMIN}), - ("grace", "lib:Org1:math_advanced", {roles.LIBRARY_CONTRIBUTOR}), + ("alice", "lib:Org1:math_101", {roles.LIBRARY_ADMIN.external_key}), + ("bob", "lib:Org1:history_201", {roles.LIBRARY_AUTHOR.external_key}), + ("eve", "lib:Org2:physics_401", {roles.LIBRARY_ADMIN.external_key}), + ("grace", "lib:Org1:math_advanced", {roles.LIBRARY_CONTRIBUTOR.external_key}), ) @unpack def test_get_user_role_assignments_in_scope(self, username, scope_name, expected_roles): @@ -148,9 +148,9 @@ def test_get_user_role_assignments_in_scope(self, username, scope_name, expected self.assertEqual(role_names, expected_roles) @data( - (roles.LIBRARY_ADMIN, "lib:Org1:math_101", {"alice"}), - (roles.LIBRARY_AUTHOR, "lib:Org1:history_201", {"bob"}), - (roles.LIBRARY_CONTRIBUTOR, "lib:Org1:math_advanced", {"grace", "heidi"}), + (roles.LIBRARY_ADMIN.external_key, "lib:Org1:math_101", {"alice"}), + (roles.LIBRARY_AUTHOR.external_key, "lib:Org1:history_201", {"bob"}), + (roles.LIBRARY_CONTRIBUTOR.external_key, "lib:Org1:math_advanced", {"grace", "heidi"}), ) @unpack def test_get_user_role_assignments_for_role_in_scope(self, role_name, scope_name, expected_users): @@ -176,8 +176,8 @@ def test_get_user_role_assignments_for_role_in_scope(self, role_name, scope_name subject=UserData(external_key="alice"), roles=[ RoleData( - external_key=roles.LIBRARY_ADMIN, - permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, + external_key=roles.LIBRARY_ADMIN.external_key, + permissions=LIBRARY_ADMIN_PERMISSIONS, ) ], scope=ContentLibraryData(external_key="lib:Org1:math_101"), @@ -191,8 +191,8 @@ def test_get_user_role_assignments_for_role_in_scope(self, role_name, scope_name subject=UserData(external_key="bob"), roles=[ RoleData( - external_key=roles.LIBRARY_AUTHOR, - permissions=LIST_LIBRARY_AUTHOR_PERMISSIONS, + external_key=roles.LIBRARY_AUTHOR.external_key, + permissions=LIBRARY_AUTHOR_PERMISSIONS, ) ], scope=ContentLibraryData(external_key="lib:Org1:history_201"), @@ -206,8 +206,8 @@ def test_get_user_role_assignments_for_role_in_scope(self, role_name, scope_name subject=UserData(external_key="eve"), roles=[ RoleData( - external_key=roles.LIBRARY_ADMIN, - permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, + external_key=roles.LIBRARY_ADMIN.external_key, + permissions=LIBRARY_ADMIN_PERMISSIONS, ) ], scope=ContentLibraryData(external_key="lib:Org2:physics_401"), @@ -235,16 +235,16 @@ class TestUserPermissions(UserAssignmentsSetupMixin): """Test suite for user permission API functions.""" @data( - ("alice", permissions.DELETE_LIBRARY, "lib:Org1:math_101", True), - ("bob", permissions.PUBLISH_LIBRARY_CONTENT, "lib:Org1:history_201", True), - ("eve", permissions.MANAGE_LIBRARY_TEAM, "lib:Org2:physics_401", True), - ("grace", permissions.EDIT_LIBRARY_CONTENT, "lib:Org1:math_advanced", True), - ("heidi", permissions.CREATE_LIBRARY_COLLECTION, "lib:Org1:math_advanced", True), - ("charlie", permissions.DELETE_LIBRARY, "lib:Org1:science_301", False), - ("david", permissions.PUBLISH_LIBRARY_CONTENT, "lib:Org1:history_201", False), - ("mallory", permissions.MANAGE_LIBRARY_TEAM, "lib:Org1:math_101", False), - ("oscar", permissions.EDIT_LIBRARY_CONTENT, "lib:Org4:art_101", False), - ("peggy", permissions.CREATE_LIBRARY_COLLECTION, "lib:Org2:physics_401", False), + ("alice", permissions.DELETE_LIBRARY.identifier, "lib:Org1:math_101", True), + ("bob", permissions.PUBLISH_LIBRARY_CONTENT.identifier, "lib:Org1:history_201", True), + ("eve", permissions.MANAGE_LIBRARY_TEAM.identifier, "lib:Org2:physics_401", True), + ("grace", permissions.EDIT_LIBRARY_CONTENT.identifier, "lib:Org1:math_advanced", True), + ("heidi", permissions.CREATE_LIBRARY_COLLECTION.identifier, "lib:Org1:math_advanced", True), + ("charlie", permissions.DELETE_LIBRARY.identifier, "lib:Org1:science_301", False), + ("david", permissions.PUBLISH_LIBRARY_CONTENT.identifier, "lib:Org1:history_201", False), + ("mallory", permissions.MANAGE_LIBRARY_TEAM.identifier, "lib:Org1:math_101", False), + ("oscar", permissions.EDIT_LIBRARY_CONTENT.identifier, "lib:Org4:art_101", False), + ("peggy", permissions.CREATE_LIBRARY_COLLECTION.identifier, "lib:Org2:physics_401", False), ) @unpack def test_is_user_allowed(self, username, action, scope_name, expected_result): diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index ff5d6b5f..c6b2d332 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -69,48 +69,48 @@ def setUpClass(cls): # Assign roles to admin users { "subject_name": "admin_1", - "role_name": roles.LIBRARY_ADMIN, + "role_name": roles.LIBRARY_ADMIN.external_key, "scope_name": "lib:Org1:LIB1", }, { "subject_name": "admin_2", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org2:LIB2", }, { "subject_name": "admin_3", - "role_name": roles.LIBRARY_ADMIN, + "role_name": roles.LIBRARY_ADMIN.external_key, "scope_name": "lib:Org3:LIB3", }, # Assign roles to regular users { "subject_name": "regular_1", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org1:LIB1", }, { "subject_name": "regular_2", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org1:LIB1", }, { "subject_name": "regular_3", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org2:LIB2", }, { "subject_name": "regular_4", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org2:LIB2", }, { "subject_name": "regular_5", - "role_name": roles.LIBRARY_ADMIN, + "role_name": roles.LIBRARY_ADMIN.external_key, "scope_name": "lib:Org3:LIB3", }, { "subject_name": "regular_6", - "role_name": roles.LIBRARY_AUTHOR, + "role_name": roles.LIBRARY_AUTHOR.external_key, "scope_name": "lib:Org3:LIB3", }, { @@ -120,7 +120,7 @@ def setUpClass(cls): }, { "subject_name": "regular_8", - "role_name": roles.LIBRARY_USER, + "role_name": roles.LIBRARY_USER.external_key, "scope_name": "lib:Org3:LIB3", }, ] @@ -165,16 +165,16 @@ def setUp(self): @data( # Single permission - allowed - ([{"action": permissions.VIEW_LIBRARY, "scope": "lib:Org1:LIB1"}], [True]), + ([{"action": permissions.VIEW_LIBRARY.identifier, "scope": "lib:Org1:LIB1"}], [True]), # Single permission - denied (scope not assigned to user) - ([{"action": permissions.VIEW_LIBRARY, "scope": "lib:Org2:LIB2"}], [False]), + ([{"action": permissions.VIEW_LIBRARY.identifier, "scope": "lib:Org2:LIB2"}], [False]), # # Single permission - denied (action not assigned to user) ([{"action": "edit_library", "scope": "lib:Org1:LIB1"}], [False]), # # Multiple permissions - mixed results ( [ - {"action": permissions.VIEW_LIBRARY, "scope": "lib:Org1:LIB1"}, - {"action": permissions.VIEW_LIBRARY, "scope": "lib:Org2:LIB2"}, + {"action": permissions.VIEW_LIBRARY.identifier, "scope": "lib:Org1:LIB1"}, + {"action": permissions.VIEW_LIBRARY.identifier, "scope": "lib:Org2:LIB2"}, {"action": "edit_library", "scope": "lib:Org1:LIB1"}, ], [True, False, False], @@ -305,23 +305,23 @@ def setUp(self): ({"search": "@example.com"}, 3), ({"search": "nonexistent@example.com"}, 0), # Search by single role - ({"roles": roles.LIBRARY_ADMIN}, 1), - ({"roles": roles.LIBRARY_AUTHOR}, 0), - ({"roles": roles.LIBRARY_USER}, 2), + ({"roles": roles.LIBRARY_ADMIN.external_key}, 1), + ({"roles": roles.LIBRARY_AUTHOR.external_key}, 0), + ({"roles": roles.LIBRARY_USER.external_key}, 2), # Search by multiple roles ({"roles": "library_admin,library_author"}, 1), ({"roles": "library_author,library_user"}, 2), ({"roles": "library_user,library_admin"}, 3), ({"roles": "library_admin,library_author,library_user"}, 3), # Search by role and username - ({"search": "admin_1", "roles": roles.LIBRARY_ADMIN}, 1), - ({"search": "regular_1", "roles": roles.LIBRARY_USER}, 1), - ({"search": "regular_1", "roles": roles.LIBRARY_ADMIN}, 0), + ({"search": "admin_1", "roles": roles.LIBRARY_ADMIN.external_key}, 1), + ({"search": "regular_1", "roles": roles.LIBRARY_USER.external_key}, 1), + ({"search": "regular_1", "roles": roles.LIBRARY_ADMIN.external_key}, 0), # Search by role and email - ({"search": "admin_1@example.com", "roles": roles.LIBRARY_ADMIN}, 1), - ({"search": "@example.com", "roles": roles.LIBRARY_ADMIN}, 1), - ({"search": "@example.com", "roles": roles.LIBRARY_USER}, 2), - ({"search": "regular_1@example.com", "roles": roles.LIBRARY_ADMIN}, 0), + ({"search": "admin_1@example.com", "roles": roles.LIBRARY_ADMIN.external_key}, 1), + ({"search": "@example.com", "roles": roles.LIBRARY_ADMIN.external_key}, 1), + ({"search": "@example.com", "roles": roles.LIBRARY_USER.external_key}, 2), + ({"search": "regular_1@example.com", "roles": roles.LIBRARY_ADMIN.external_key}, 0), ) @unpack def test_get_users_by_scope_success(self, query_params: dict, expected_count: int): @@ -435,7 +435,7 @@ def test_add_users_to_role_success(self, users: list[str], expected_completed: i - Returns 207 MULTI-STATUS status - Returns appropriate completed and error counts """ - role = roles.LIBRARY_ADMIN + role = roles.LIBRARY_ADMIN.external_key request_data = {"role": role, "scope": "lib:Org1:LIB3", "users": users} with patch.object(api.ContentLibraryData, "exists", return_value=True): @@ -458,7 +458,7 @@ def test_add_users_to_role_success(self, users: list[str], expected_completed: i @unpack def test_add_users_to_role_already_has_role(self, users: list[str], expected_completed: int, expected_errors: int): """Test adding users to a role that already has the role.""" - role = roles.LIBRARY_USER + role = roles.LIBRARY_USER.external_key scope = "lib:Org2:LIB2" request_data = {"role": role, "scope": scope, "users": users} @@ -473,7 +473,7 @@ def test_add_users_to_role_already_has_role(self, users: list[str], expected_com def test_add_users_to_role_exception_handling(self, mock_assign_role_to_user_in_scope): """Test adding users to a role with exception handling.""" request_data = { - "role": roles.LIBRARY_ADMIN, + "role": roles.LIBRARY_ADMIN.external_key, "scope": "lib:Org1:LIB1", "users": ["regular_1"], } @@ -493,15 +493,15 @@ def test_add_users_to_role_exception_handling(self, mock_assign_role_to_user_in_ @data( {}, - {"role": roles.LIBRARY_ADMIN}, + {"role": roles.LIBRARY_ADMIN.external_key}, {"scope": "lib:Org1:LIB1"}, {"users": ["admin_1"]}, - {"role": roles.LIBRARY_ADMIN, "scope": "lib:Org1:LIB1"}, + {"role": roles.LIBRARY_ADMIN.external_key, "scope": "lib:Org1:LIB1"}, {"scope": "lib:Org1:LIB1", "users": ["admin_1"]}, - {"users": ["admin_1", "regular_1"], "role": roles.LIBRARY_ADMIN}, - {"role": roles.LIBRARY_ADMIN, "scope": "lib:Org1:LIB1", "users": []}, + {"users": ["admin_1", "regular_1"], "role": roles.LIBRARY_ADMIN.external_key}, + {"role": roles.LIBRARY_ADMIN.external_key, "scope": "lib:Org1:LIB1", "users": []}, {"role": "", "scope": "lib:Org1:LIB1", "users": ["admin_1"]}, - {"role": roles.LIBRARY_ADMIN, "scope": "", "users": ["admin_1"]}, + {"role": roles.LIBRARY_ADMIN.external_key, "scope": "", "users": ["admin_1"]}, ) def test_add_users_to_role_invalid_data(self, request_data: dict): """Test adding users with invalid request data. @@ -532,7 +532,7 @@ def test_add_users_to_role_permissions(self, username: str, status_code: int): - Returns appropriate status code based on permissions """ request_data = { - "role": roles.LIBRARY_ADMIN, + "role": roles.LIBRARY_ADMIN.external_key, "scope": "lib:Org3:LIB3", "users": ["regular_2"], } @@ -587,7 +587,7 @@ def test_remove_users_from_role_success(self, users: list[str], expected_complet - Returns appropriate completed and error counts """ query_params = { - "role": roles.LIBRARY_USER, + "role": roles.LIBRARY_USER.external_key, "scope": "lib:Org2:LIB2", "users": ",".join(users), } @@ -603,7 +603,7 @@ def test_remove_users_from_role_success(self, users: list[str], expected_complet def test_remove_users_from_role_exception_handling(self, mock_unassign_role_from_user): """Test removing users from a role with exception handling.""" query_params = { - "role": roles.LIBRARY_ADMIN, + "role": roles.LIBRARY_ADMIN.external_key, "scope": "lib:Org1:LIB1", "users": "regular_1,regular_2,regular_3", } @@ -632,15 +632,15 @@ def test_remove_users_from_role_exception_handling(self, mock_unassign_role_from @data( {}, - {"role": roles.LIBRARY_ADMIN}, + {"role": roles.LIBRARY_ADMIN.external_key}, {"scope": "lib:Org1:LIB1"}, {"users": "admin_1"}, - {"role": roles.LIBRARY_ADMIN, "scope": "lib:Org1:LIB1"}, + {"role": roles.LIBRARY_ADMIN.external_key, "scope": "lib:Org1:LIB1"}, {"scope": "lib:Org1:LIB1", "users": "admin_1"}, - {"users": "admin_1,regular_1", "role": roles.LIBRARY_ADMIN}, - {"role": roles.LIBRARY_ADMIN, "scope": "lib:Org1:LIB1", "users": ""}, + {"users": "admin_1,regular_1", "role": roles.LIBRARY_ADMIN.external_key}, + {"role": roles.LIBRARY_ADMIN.external_key, "scope": "lib:Org1:LIB1", "users": ""}, {"role": "", "scope": "lib:Org1:LIB1", "users": "admin_1"}, - {"role": roles.LIBRARY_ADMIN, "scope": "", "users": "admin_1"}, + {"role": roles.LIBRARY_ADMIN.external_key, "scope": "", "users": "admin_1"}, ) def test_remove_users_from_role_invalid_params(self, query_params: dict): """Test removing users with invalid query parameters. @@ -670,7 +670,7 @@ def test_remove_users_from_role_permissions(self, username: str, status_code: in - Returns appropriate status code based on permissions """ query_params = { - "role": roles.LIBRARY_ADMIN, + "role": roles.LIBRARY_ADMIN.external_key, "scope": "lib:Org3:LIB3", "users": "user1,user2", } diff --git a/openedx_authz/tests/test_commands.py b/openedx_authz/tests/test_commands.py index 7e164adf..7d444867 100644 --- a/openedx_authz/tests/test_commands.py +++ b/openedx_authz/tests/test_commands.py @@ -141,7 +141,7 @@ def test_interactive_mode_allowed_request(self, mock_is_allowed: Mock, mock_get_ output = self.buffer.getvalue() self.assertIn("✓ ALLOWED: alice view_library lib:Org1:LIB1", output) - mock_is_allowed.assert_called_once_with("alice", permissions.VIEW_LIBRARY, "lib:Org1:LIB1") + mock_is_allowed.assert_called_once_with("alice", permissions.VIEW_LIBRARY.identifier, "lib:Org1:LIB1") @patch.object(AuthzEnforcer, "get_enforcer") @patch.object(authz_api, "is_user_allowed") @@ -155,7 +155,7 @@ def test_interactive_mode_denied_request(self, mock_is_allowed: Mock, mock_get_e output = self.buffer.getvalue() self.assertIn("✗ DENIED: bob delete_library lib:Org2:LIB2", output) - mock_is_allowed.assert_called_once_with("bob", permissions.DELETE_LIBRARY, "lib:Org2:LIB2") + mock_is_allowed.assert_called_once_with("bob", permissions.DELETE_LIBRARY.identifier, "lib:Org2:LIB2") @patch("openedx_authz.management.commands.enforcement.Enforcer") def test_interactive_mode_file_mode_enforcement(self, mock_enforcer_class: Mock): diff --git a/openedx_authz/tests/test_enforcement.py b/openedx_authz/tests/test_enforcement.py index dbbd65e3..ccb2144e 100644 --- a/openedx_authz/tests/test_enforcement.py +++ b/openedx_authz/tests/test_enforcement.py @@ -258,7 +258,7 @@ class RoleAssignmentTests(CasbinEnforcementTestCase): ], [ "p", - make_role_key(roles.LIBRARY_ADMIN), + make_role_key(roles.LIBRARY_ADMIN.external_key), make_action_key("manage"), make_scope_key("lib", "*"), "allow", @@ -279,7 +279,7 @@ class RoleAssignmentTests(CasbinEnforcementTestCase): ], [ "p", - make_role_key(roles.LIBRARY_AUTHOR), + make_role_key(roles.LIBRARY_AUTHOR.external_key), make_action_key("write"), make_scope_key("lib", "*"), "allow", @@ -313,7 +313,7 @@ class RoleAssignmentTests(CasbinEnforcementTestCase): [ "g", make_user_key("user-6"), - make_role_key(roles.LIBRARY_ADMIN), + make_role_key(roles.LIBRARY_ADMIN.external_key), make_library_key("lib:DemoX:CSPROB"), ], [ @@ -331,7 +331,7 @@ class RoleAssignmentTests(CasbinEnforcementTestCase): [ "g", make_user_key("user-9"), - make_role_key(roles.LIBRARY_AUTHOR), + make_role_key(roles.LIBRARY_AUTHOR.external_key), make_library_key("lib:DemoX:CSPROB"), ], ] + COMMON_ACTION_GROUPING @@ -494,7 +494,7 @@ class WildcardScopeTests(CasbinEnforcementTestCase): ], [ "p", - make_role_key(roles.LIBRARY_ADMIN), + make_role_key(roles.LIBRARY_ADMIN.external_key), make_action_key("manage"), make_scope_key("lib", "*"), "allow", @@ -503,7 +503,7 @@ class WildcardScopeTests(CasbinEnforcementTestCase): ["g", make_user_key("user-1"), make_role_key("platform_admin"), "*"], ["g", make_user_key("user-2"), make_role_key("org_admin"), "*"], ["g", make_user_key("user-3"), make_role_key("course_admin"), "*"], - ["g", make_user_key("user-4"), make_role_key(roles.LIBRARY_ADMIN), "*"], + ["g", make_user_key("user-4"), make_role_key(roles.LIBRARY_ADMIN.external_key), "*"], ] + COMMON_ACTION_GROUPING @data( diff --git a/openedx_authz/tests/test_engine_utils.py b/openedx_authz/tests/test_engine_utils.py index 7ec3118e..6908e831 100644 --- a/openedx_authz/tests/test_engine_utils.py +++ b/openedx_authz/tests/test_engine_utils.py @@ -93,8 +93,8 @@ def test_migrate_all_file_policies_to_database(self): self.assertIn( [ - make_role_key(roles.LIBRARY_ADMIN), - make_action_key(permissions.DELETE_LIBRARY), + make_role_key(roles.LIBRARY_ADMIN.external_key), + make_action_key(permissions.DELETE_LIBRARY.identifier), make_scope_key("lib", "*"), "allow", ], @@ -102,8 +102,8 @@ def test_migrate_all_file_policies_to_database(self): ) self.assertIn( [ - make_role_key(roles.LIBRARY_USER), - make_action_key(permissions.VIEW_LIBRARY), + make_role_key(roles.LIBRARY_USER.external_key), + make_action_key(permissions.VIEW_LIBRARY.identifier), make_scope_key("lib", "*"), "allow", ], @@ -152,15 +152,15 @@ def test_migrate_action_inheritance_from_file(self): # Verify a sample of expected g2 rules from the file self.assertIn( [ - make_action_key(permissions.DELETE_LIBRARY), - make_action_key(permissions.EDIT_LIBRARY_CONTENT), + make_action_key(permissions.DELETE_LIBRARY.identifier), + make_action_key(permissions.EDIT_LIBRARY_CONTENT.identifier), ], target_g2, ) self.assertIn( [ - make_action_key(permissions.MANAGE_LIBRARY_TEAM), - make_action_key(permissions.VIEW_LIBRARY_TEAM), + make_action_key(permissions.MANAGE_LIBRARY_TEAM.identifier), + make_action_key(permissions.VIEW_LIBRARY_TEAM.identifier), ], target_g2, ) @@ -239,8 +239,8 @@ def test_migrate_partial_duplicates(self): - Mixed state is handled correctly """ self.target_enforcer.add_policy( - make_role_key(roles.LIBRARY_ADMIN), - make_action_key(permissions.DELETE_LIBRARY), + make_role_key(roles.LIBRARY_ADMIN.external_key), + make_action_key(permissions.DELETE_LIBRARY.identifier), make_scope_key("lib", "*"), "allow", ) @@ -264,18 +264,18 @@ def test_migrate_partial_duplicates(self): @data( ( - make_role_key(roles.LIBRARY_ADMIN), - make_action_key(permissions.DELETE_LIBRARY), + make_role_key(roles.LIBRARY_ADMIN.external_key), + make_action_key(permissions.DELETE_LIBRARY.identifier), make_scope_key("lib", "*"), ), ( - make_role_key(roles.LIBRARY_USER), - make_action_key(permissions.VIEW_LIBRARY), + make_role_key(roles.LIBRARY_USER.external_key), + make_action_key(permissions.VIEW_LIBRARY.identifier), make_scope_key("lib", "*"), ), ( - make_role_key(roles.LIBRARY_AUTHOR), - make_action_key(permissions.EDIT_LIBRARY_CONTENT), + make_role_key(roles.LIBRARY_AUTHOR.external_key), + make_action_key(permissions.EDIT_LIBRARY_CONTENT.identifier), make_scope_key("lib", "*"), ), ) @@ -297,9 +297,18 @@ def test_migrate_specific_file_policies(self, role, action, scope): ) @data( - (make_action_key(permissions.DELETE_LIBRARY), make_action_key(permissions.EDIT_LIBRARY_CONTENT)), - (make_action_key(permissions.EDIT_LIBRARY_CONTENT), make_action_key(permissions.VIEW_LIBRARY)), - (make_action_key(permissions.MANAGE_LIBRARY_TEAM), make_action_key(permissions.VIEW_LIBRARY_TEAM)), + ( + make_action_key(permissions.DELETE_LIBRARY.identifier), + make_action_key(permissions.EDIT_LIBRARY_CONTENT.identifier), + ), + ( + make_action_key(permissions.EDIT_LIBRARY_CONTENT.identifier), + make_action_key(permissions.VIEW_LIBRARY.identifier), + ), + ( + make_action_key(permissions.MANAGE_LIBRARY_TEAM.identifier), + make_action_key(permissions.VIEW_LIBRARY_TEAM.identifier), + ), ) @unpack def test_migrate_specific_action_inheritance(self, parent_action, child_action): @@ -350,12 +359,12 @@ def test_migrate_preserves_user_role_assignments_in_db(self): """ self.target_enforcer.add_grouping_policy( make_user_key("user-1"), - make_role_key(roles.LIBRARY_ADMIN), + make_role_key(roles.LIBRARY_ADMIN.external_key), make_scope_key("lib", "demo"), ) self.target_enforcer.add_grouping_policy( make_user_key("user-2"), - make_role_key(roles.LIBRARY_USER), + make_role_key(roles.LIBRARY_USER.external_key), make_scope_key("lib", "*"), ) @@ -366,7 +375,7 @@ def test_migrate_preserves_user_role_assignments_in_db(self): self.assertIn( [ make_user_key("user-1"), - make_role_key(roles.LIBRARY_ADMIN), + make_role_key(roles.LIBRARY_ADMIN.external_key), make_scope_key("lib", "demo"), ], target_grouping,