From d00af9a0d51cc3e001fe80c2c19b776b8005e503 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Thu, 9 Oct 2025 19:34:00 -0500 Subject: [PATCH 1/5] feat: add default policy --- openedx_authz/engine/config/authz.policy | 54 +++++++++++-------- .../management/commands/load_policies.py | 10 +++- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/openedx_authz/engine/config/authz.policy b/openedx_authz/engine/config/authz.policy index 6c2c6759..c65ef4a0 100644 --- a/openedx_authz/engine/config/authz.policy +++ b/openedx_authz/engine/config/authz.policy @@ -9,52 +9,62 @@ # For role definitions use: lib^*, course^*, org^* to specify the scope of the role # Library Admin Role Policies -p, role^library_admin, act^delete_library, lib^*, allow -p, role^library_admin, act^publish_library, lib^*, allow -p, role^library_admin, act^manage_library_team, lib^*, allow +p, role^library_admin, act^view_library, lib^*, allow p, role^library_admin, act^manage_library_tags, lib^*, allow -p, role^library_admin, act^delete_library_content, lib^*, allow +p, role^library_admin, act^delete_library, lib^*, allow +p, role^library_admin, act^edit_library_content, lib^*, allow p, role^library_admin, act^publish_library_content, lib^*, allow -p, role^library_admin, act^delete_library_collection, lib^*, allow -p, role^library_admin, act^create_library, lib^*, allow +p, role^library_admin, act^reuse_library_content, lib^*, allow +p, role^library_admin, act^view_library_team, lib^*, allow +p, role^library_admin, act^manage_library_team, lib^*, allow p, role^library_admin, act^create_library_collection, lib^*, allow +p, role^library_admin, act^edit_library_collection, lib^*, allow +p, role^library_admin, act^delete_library_collection, lib^*, allow # Library Author Role Policies -p, role^library_author, act^delete_library_content, lib^*, allow -p, role^library_author, act^publish_library_content, lib^*, allow -p, role^library_author, act^edit_library, lib^*, allow +p, role^library_author, act^view_library, lib^*, allow p, role^library_author, act^manage_library_tags, lib^*, allow +p, role^library_author, act^edit_library_content, lib^*, allow +p, role^library_author, act^publish_library_content, lib^*, allow +p, role^library_author, act^reuse_library_content, lib^*, allow +p, role^library_author, act^view_library_team, lib^*, allow p, role^library_author, act^create_library_collection, lib^*, allow p, role^library_author, act^edit_library_collection, lib^*, allow p, role^library_author, act^delete_library_collection, lib^*, allow # Library Collaborator Role Policies -p, role^library_collaborator, act^edit_library, lib^*, allow -p, role^library_collaborator, act^delete_library_content, lib^*, allow +p, role^library_collaborator, act^view_library, lib^*, allow p, role^library_collaborator, act^manage_library_tags, lib^*, allow +p, role^library_collaborator, act^edit_library_content, lib^*, allow +p, role^library_collaborator, act^reuse_library_content, lib^*, allow +p, role^library_collaborator, act^view_library_team, lib^*, allow p, role^library_collaborator, act^create_library_collection, lib^*, allow p, role^library_collaborator, act^edit_library_collection, lib^*, allow p, role^library_collaborator, act^delete_library_collection, lib^*, allow # Library User Role Policies p, role^library_user, act^view_library, lib^*, allow -p, role^library_user, act^view_library_team, lib^*, allow p, role^library_user, act^reuse_library_content, lib^*, allow +p, role^library_user, act^view_library_team, lib^*, allow # Action Inheritance (g2) - format: g2 = granted_action, implied_action # Higher-level permissions automatically grant lower-level permissions # If a user has the granted_action, they also have the implied_action # Example: g2, act^delete_library, act^view_library means delete permission includes view permission -g2, act^delete_library, act^view_library -g2, act^edit_library, act^view_library -g2, act^create_library, act^view_library -g2, act^publish_library, act^view_library +# Library +g2, act^manage_library_tags, act^edit_library_content +g2, act^delete_library, act^edit_library_content + +# Content +g2, act^publish_library_content, act^edit_library_content +g2, act^edit_library_content, act^view_library +g2, act^reuse_library_content, act^view_library +g2, act^publish_library_content, act^view_library + +# Team g2, act^manage_library_team, act^view_library_team -g2, act^manage_library_tags, act^view_library_tags + +# Collections g2, act^delete_library_collection, act^edit_library_collection -g2, act^edit_library_collection, act^view_library_collection g2, act^create_library_collection, act^edit_library_collection -g2, act^edit_library_content, act^view_library_content -g2, act^delete_library_content, act^edit_library_content -g2, act^publish_library_content, act^view_library_content -g2, act^reuse_library_content, act^view_library_content +g2, act^edit_library_collection, act^view_library diff --git a/openedx_authz/management/commands/load_policies.py b/openedx_authz/management/commands/load_policies.py index 36d34ad6..13af5cd7 100644 --- a/openedx_authz/management/commands/load_policies.py +++ b/openedx_authz/management/commands/load_policies.py @@ -11,11 +11,11 @@ import casbin from django.core.management.base import BaseCommand +from casbin_adapter.models import CasbinRule from openedx_authz import ROOT_DIRECTORY from openedx_authz.engine.enforcer import enforcer as global_enforcer from openedx_authz.engine.utils import migrate_policy_between_enforcers - class Command(BaseCommand): """Django management command to load policies into the authorization Django model. @@ -49,6 +49,11 @@ def add_arguments(self, parser) -> None: default=None, help="Path to the Casbin model configuration file", ) + parser.add_argument( + "--clear-existing", + action="store_true", + help="Flag to clear existing policies before loading new ones", + ) def handle(self, *args, **options): """Execute the policy loading command. @@ -73,6 +78,9 @@ def handle(self, *args, **options): ROOT_DIRECTORY, "engine", "config", "model.conf" ) + if options.get("clear_existing"): + CasbinRule.objects.all().delete() + source_enforcer = casbin.Enforcer(model_file_path, policy_file_path) self.migrate_policies(source_enforcer, global_enforcer) From 15dc890e3c9c376b9ab9e3e08e8ac916567a65c8 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Fri, 10 Oct 2025 19:25:04 -0500 Subject: [PATCH 2/5] test: fix and refactor api tests --- .../management/commands/load_policies.py | 1 + openedx_authz/tests/api/test_roles.py | 476 +----------------- openedx_authz/tests/api/test_users.py | 145 +----- openedx_authz/tests/constants.py | 144 ++++++ 4 files changed, 176 insertions(+), 590 deletions(-) create mode 100644 openedx_authz/tests/constants.py diff --git a/openedx_authz/management/commands/load_policies.py b/openedx_authz/management/commands/load_policies.py index 13af5cd7..52091398 100644 --- a/openedx_authz/management/commands/load_policies.py +++ b/openedx_authz/management/commands/load_policies.py @@ -16,6 +16,7 @@ from openedx_authz.engine.enforcer import enforcer as global_enforcer from openedx_authz.engine.utils import migrate_policy_between_enforcers + class Command(BaseCommand): """Django management command to load policies into the authorization Django model. diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index 6a8df637..9b3e350b 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -11,9 +11,7 @@ from django.test import TestCase from openedx_authz.api.data import ( - ActionData, ContentLibraryData, - PermissionData, RoleAssignmentData, RoleData, ScopeData, @@ -33,6 +31,12 @@ ) from openedx_authz.engine.enforcer import enforcer as global_enforcer from openedx_authz.engine.utils import migrate_policy_between_enforcers +from openedx_authz.tests.constants import ( + LIST_LIBRARY_ADMIN_PERMISSIONS, + LIST_LIBRARY_AUTHOR_PERMISSIONS, + LIST_LIBRARY_COLLABORATOR_PERMISSIONS, + LIST_LIBRARY_USER_PERMISSIONS, +) class BaseRolesTestCase(TestCase): @@ -266,126 +270,22 @@ class TestRolesAPI(RolesTestSetupMixin): # Library Admin role with actual permissions from authz.policy ( "library_admin", - [ - PermissionData( - action=ActionData(external_key="delete_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_collection"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library_collection"), - effect="allow", - ), - ], + LIST_LIBRARY_ADMIN_PERMISSIONS, ), # Library Author role with actual permissions from authz.policy ( "library_author", - [ - PermissionData( - action=ActionData(external_key="delete_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - 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, ), # Library Collaborator role with actual permissions from authz.policy ( "library_collaborator", - [ - PermissionData( - action=ActionData(external_key="edit_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - 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_COLLABORATOR_PERMISSIONS, ), # Library User role with minimal permissions ( "library_user", - [ - PermissionData( - action=ActionData(external_key="view_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="view_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="reuse_library_content"), - effect="allow", - ), - ], + LIST_LIBRARY_USER_PERMISSIONS, ), # Non existent role ( @@ -412,92 +312,19 @@ def test_get_permissions_for_roles(self, role_name, expected_permissions): ( "library_user", "lib:Org1:english_101", - [ - PermissionData( - action=ActionData(external_key="view_library"), effect="allow" - ), - PermissionData( - action=ActionData(external_key="view_library_team"), effect="allow" - ), - PermissionData( - action=ActionData(external_key="reuse_library_content"), - effect="allow", - ), - ], + LIST_LIBRARY_USER_PERMISSIONS, ), # Role assigned to single user in single scope ( "library_author", "lib:Org1:history_201", - [ - PermissionData( - action=ActionData(external_key="delete_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library"), effect="allow" - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - 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, ), # Role assigned to single user in multiple scopes ( "library_admin", "lib:Org1:math_101", - [ - PermissionData( - action=ActionData(external_key="delete_library"), effect="allow" - ), - PermissionData( - action=ActionData(external_key="publish_library"), effect="allow" - ), - PermissionData( - action=ActionData(external_key="manage_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_collection"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library"), effect="allow" - ), - PermissionData( - action=ActionData(external_key="create_library_collection"), - effect="allow", - ), - ], + LIST_LIBRARY_ADMIN_PERMISSIONS, ), ) @unpack @@ -598,44 +425,7 @@ def test_get_subject_role_assignments_in_scope( [ RoleData( external_key="library_admin", - permissions=[ - PermissionData( - action=ActionData(external_key="delete_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_collection"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library_collection"), - effect="allow", - ), - ], + permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, ), ], ), @@ -644,94 +434,15 @@ def test_get_subject_role_assignments_in_scope( [ RoleData( external_key="library_admin", - permissions=[ - PermissionData( - action=ActionData(external_key="delete_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="delete_library_collection"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library_collection"), - effect="allow", - ), - ], + permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, ), RoleData( external_key="library_author", - permissions=[ - PermissionData( - action=ActionData(external_key="delete_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library_content"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - 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", - ), - ], + permissions=LIST_LIBRARY_AUTHOR_PERMISSIONS, ), RoleData( external_key="library_user", - permissions=[ - PermissionData( - action=ActionData(external_key="view_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="view_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="reuse_library_content"), - effect="allow", - ), - ], + permissions=LIST_LIBRARY_USER_PERMISSIONS, ), ], ), @@ -740,20 +451,7 @@ def test_get_subject_role_assignments_in_scope( [ RoleData( external_key="library_user", - permissions=[ - PermissionData( - action=ActionData(external_key="view_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="view_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="reuse_library_content"), - effect="allow", - ), - ], + permissions=LIST_LIBRARY_USER_PERMISSIONS, ), ], ), @@ -957,52 +655,7 @@ def test_unassign_role_from_subject_in_scope( subject=SubjectData(external_key="alice"), roles=[RoleData( external_key="library_admin", - permissions=[ - PermissionData( - action=ActionData(external_key="delete_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="delete_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="publish_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="delete_library_collection" - ), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library"), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="create_library_collection" - ), - effect="allow", - ), - ], + permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, )], scope=ScopeData(external_key="lib:Org1:math_101"), ) @@ -1015,46 +668,7 @@ def test_unassign_role_from_subject_in_scope( subject=SubjectData(external_key="bob"), roles=[RoleData( external_key="library_author", - permissions=[ - PermissionData( - action=ActionData( - external_key="delete_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="publish_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - 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", - ), - ], + permissions=LIST_LIBRARY_AUTHOR_PERMISSIONS, )], scope=ScopeData(external_key="lib:Org1:history_201"), ) @@ -1067,40 +681,7 @@ def test_unassign_role_from_subject_in_scope( subject=SubjectData(external_key="carol"), roles=[RoleData( external_key="library_collaborator", - permissions=[ - PermissionData( - action=ActionData(external_key="edit_library"), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="delete_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - 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", - ), - ], + permissions=LIST_LIBRARY_COLLABORATOR_PERMISSIONS, )], scope=ScopeData(external_key="lib:Org1:science_301"), ) @@ -1113,20 +694,7 @@ def test_unassign_role_from_subject_in_scope( subject=SubjectData(external_key="dave"), roles=[RoleData( external_key="library_user", - permissions=[ - PermissionData( - action=ActionData(external_key="view_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="view_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="reuse_library_content"), - effect="allow", - ), - ], + permissions=LIST_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 32b7d85a..d010738d 100644 --- a/openedx_authz/tests/api/test_users.py +++ b/openedx_authz/tests/api/test_users.py @@ -3,9 +3,7 @@ from ddt import data, ddt, unpack from openedx_authz.api.data import ( - ActionData, ContentLibraryData, - PermissionData, RoleAssignmentData, RoleData, UserData, @@ -22,6 +20,10 @@ unassign_role_from_user, ) 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): @@ -203,52 +205,7 @@ def test_get_user_role_assignments_for_role_in_scope( subject=UserData(external_key="alice"), roles=[RoleData( external_key="library_admin", - permissions=[ - PermissionData( - action=ActionData(external_key="delete_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="delete_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="publish_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="delete_library_collection" - ), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library"), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="create_library_collection" - ), - effect="allow", - ), - ], + permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, )], scope=ContentLibraryData(external_key="lib:Org1:math_101"), ), @@ -261,46 +218,7 @@ def test_get_user_role_assignments_for_role_in_scope( subject=UserData(external_key="bob"), roles=[RoleData( external_key="library_author", - permissions=[ - PermissionData( - action=ActionData( - external_key="delete_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="publish_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="edit_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - 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", - ), - ], + permissions=LIST_LIBRARY_AUTHOR_PERMISSIONS, )], scope=ContentLibraryData(external_key="lib:Org1:history_201"), ), @@ -313,52 +231,7 @@ def test_get_user_role_assignments_for_role_in_scope( subject=UserData(external_key="eve"), roles=[RoleData( external_key="library_admin", - permissions=[ - PermissionData( - action=ActionData(external_key="delete_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="publish_library"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_team"), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="manage_library_tags"), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="delete_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="publish_library_content" - ), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="delete_library_collection" - ), - effect="allow", - ), - PermissionData( - action=ActionData(external_key="create_library"), - effect="allow", - ), - PermissionData( - action=ActionData( - external_key="create_library_collection" - ), - effect="allow", - ), - ], + permissions=LIST_LIBRARY_ADMIN_PERMISSIONS, )], scope=ContentLibraryData(external_key="lib:Org2:physics_401"), ), @@ -392,12 +265,12 @@ class TestUserPermissions(UserAssignmentsSetupMixin): ("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", "lib:Org1:math_advanced", 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", "lib:Org4:art_101", False), + ("oscar", "edit_library_content", "lib:Org4:art_101", False), ("peggy", "create_library_collection", "lib:Org2:physics_401", False), ) @unpack diff --git a/openedx_authz/tests/constants.py b/openedx_authz/tests/constants.py new file mode 100644 index 00000000..94aa19fa --- /dev/null +++ b/openedx_authz/tests/constants.py @@ -0,0 +1,144 @@ +""" +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_COLLABORATOR_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 428994decc2c550181710ea0fc76973a5e5ff843 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Mon, 13 Oct 2025 15:12:47 -0500 Subject: [PATCH 3/5] fix: isort issue --- openedx_authz/management/commands/load_policies.py | 2 +- openedx_authz/tests/api/test_roles.py | 8 +------- openedx_authz/tests/api/test_users.py | 12 ++---------- openedx_authz/tests/constants.py | 5 +---- 4 files changed, 5 insertions(+), 22 deletions(-) diff --git a/openedx_authz/management/commands/load_policies.py b/openedx_authz/management/commands/load_policies.py index 52091398..60b5ee15 100644 --- a/openedx_authz/management/commands/load_policies.py +++ b/openedx_authz/management/commands/load_policies.py @@ -9,9 +9,9 @@ import os import casbin +from casbin_adapter.models import CasbinRule from django.core.management.base import BaseCommand -from casbin_adapter.models import CasbinRule from openedx_authz import ROOT_DIRECTORY from openedx_authz.engine.enforcer import enforcer as global_enforcer from openedx_authz.engine.utils import migrate_policy_between_enforcers diff --git a/openedx_authz/tests/api/test_roles.py b/openedx_authz/tests/api/test_roles.py index 9b3e350b..c3b02b15 100644 --- a/openedx_authz/tests/api/test_roles.py +++ b/openedx_authz/tests/api/test_roles.py @@ -10,13 +10,7 @@ from ddt import ddt, unpack from django.test import TestCase -from openedx_authz.api.data import ( - ContentLibraryData, - RoleAssignmentData, - RoleData, - ScopeData, - SubjectData, -) +from openedx_authz.api.data import ContentLibraryData, RoleAssignmentData, RoleData, ScopeData, SubjectData from openedx_authz.api.roles import ( assign_role_to_subject_in_scope, batch_assign_role_to_subjects_in_scope, diff --git a/openedx_authz/tests/api/test_users.py b/openedx_authz/tests/api/test_users.py index d010738d..8e19946a 100644 --- a/openedx_authz/tests/api/test_users.py +++ b/openedx_authz/tests/api/test_users.py @@ -2,12 +2,7 @@ from ddt import data, ddt, unpack -from openedx_authz.api.data import ( - ContentLibraryData, - RoleAssignmentData, - RoleData, - UserData, -) +from openedx_authz.api.data import ContentLibraryData, RoleAssignmentData, RoleData, UserData from openedx_authz.api.users import ( assign_role_to_user_in_scope, batch_assign_role_to_users_in_scope, @@ -20,10 +15,7 @@ unassign_role_from_user, ) from openedx_authz.tests.api.test_roles import RolesTestSetupMixin -from openedx_authz.tests.constants import ( - LIST_LIBRARY_ADMIN_PERMISSIONS, - LIST_LIBRARY_AUTHOR_PERMISSIONS, -) +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 index 94aa19fa..f32a35a4 100644 --- a/openedx_authz/tests/constants.py +++ b/openedx_authz/tests/constants.py @@ -2,10 +2,7 @@ Constants for library roles and permissions tests. """ -from openedx_authz.api.data import ( - ActionData, - PermissionData, -) +from openedx_authz.api.data import ActionData, PermissionData LIST_LIBRARY_ADMIN_PERMISSIONS = [ PermissionData( From 68776e0b28b2935300330ed3b7b94fa0ee476cb0 Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Mon, 13 Oct 2025 16:06:23 -0500 Subject: [PATCH 4/5] test: add test for the command --- openedx_authz/tests/test_commands.py | 45 ++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/openedx_authz/tests/test_commands.py b/openedx_authz/tests/test_commands.py index 75240d2e..4f7ee0a5 100644 --- a/openedx_authz/tests/test_commands.py +++ b/openedx_authz/tests/test_commands.py @@ -194,3 +194,48 @@ def test_interactive_request_error(self, exception: Exception): error_output = self.buffer.getvalue() self.assertIn(f"✗ Error processing request: {str(exception)}", error_output) + + def test_load_policies_with_clear_existing_flag(self): + """Test that load_policies command clears existing policies when --clear-existing flag is used.""" + with patch('openedx_authz.management.commands.load_policies.CasbinRule') as mock_casbin_rule: + with patch('openedx_authz.management.commands.load_policies.casbin.Enforcer') as mock_enforcer_cls: + with patch( + 'openedx_authz.management.commands.load_policies.migrate_policy_between_enforcers' + ) as mock_migrate: + # Setup mocks + mock_casbin_rule.objects.all.return_value.delete.return_value = 5 # Simulate 5 deleted policies + + # Call command with --clear-existing flag + call_command( + "load_policies", + policy_file_path=self.policy_file_path.name, + clear_existing=True, + stdout=self.buffer + ) + + # Verify that existing policies were cleared + mock_casbin_rule.objects.all.assert_called_once() + mock_casbin_rule.objects.all.return_value.delete.assert_called_once() + # Verify enforcer was created and policies migrated + mock_enforcer_cls.assert_called_once() + mock_migrate.assert_called_once() + + def test_load_policies_without_clear_existing_flag(self): + """Test that load_policies command does not clear existing policies when --clear-existing flag is not used.""" + with patch('openedx_authz.management.commands.load_policies.CasbinRule') as mock_casbin_rule: + with patch('openedx_authz.management.commands.load_policies.casbin.Enforcer') as mock_enforcer_cls: + with patch( + 'openedx_authz.management.commands.load_policies.migrate_policy_between_enforcers' + ) as mock_migrate: + # Call command without --clear-existing flag + call_command( + "load_policies", + policy_file_path=self.policy_file_path.name, + stdout=self.buffer + ) + + # Verify that existing policies were NOT cleared + mock_casbin_rule.objects.all.assert_not_called() + # Verify enforcer was created and policies migrated + mock_enforcer_cls.assert_called_once() + mock_migrate.assert_called_once() From 905e51c22f46fc74d9065ee1c5da765b8c236fba Mon Sep 17 00:00:00 2001 From: Maria Fernanda Magallanes Zubillaga Date: Thu, 16 Oct 2025 19:19:58 -0500 Subject: [PATCH 5/5] feat: add the base of attribute checking --- .../engine/config/casbin_utilities.py | 53 +++++++++++++++++++ openedx_authz/engine/config/model.conf | 2 +- openedx_authz/engine/enforcer.py | 2 + openedx_authz/settings/common.py | 3 ++ 4 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 openedx_authz/engine/config/casbin_utilities.py diff --git a/openedx_authz/engine/config/casbin_utilities.py b/openedx_authz/engine/config/casbin_utilities.py new file mode 100644 index 00000000..530c2070 --- /dev/null +++ b/openedx_authz/engine/config/casbin_utilities.py @@ -0,0 +1,53 @@ +# In your openedx-authz/casbin_utils.py + +from django.contrib.auth.models import User + +from openedx.core.djangoapps.content_libraries.models import ContentLibrary +from cms.djangoapps.course_creators.views import get_course_creator_status +from opaque_keys.edx.locator import LibraryLocatorV2 + + +def check_custom_conditions(request_user, request_action, request_scope): + """ + Evaluates custom, non-role-based conditions using Bridgekeeper logic. + """ + # Check if user exists + try: + user_id = request_user.split('^')[-1] + user = User.objects.get(username=user_id) + except User.DoesNotExist: + return False + + try: + if request_scope != "*": + scope_type = request_scope.split('^')[0] + resource_id = request_scope.split('^')[-1] + # Check if library exists + if scope_type == "lib": + try: + resource_parts = resource_id.split(':') + if len(resource_parts) < 3: + return False + org = resource_parts[1] + slug = resource_parts[2] + library_key = LibraryLocatorV2(org=org, slug=slug) + library = ContentLibrary.objects.get_by_key(library_key) + except ContentLibrary.DoesNotExist: + return False + except IndexError: + return False + + # Global Staff Check + # This is a general, high-privilege override + if user.is_staff: + return True # Global staff is always allowed + + # 3. Fine-Grained, Action-Specific Checks + # For more complex checks, you define the logic here using Bridgekeeper syntax + if request_action == "act^create_library": + return get_course_creator_status(user) == 'granted' + + elif request_action == "act^view_library": + return library.allow_public_read + else: + return False \ No newline at end of file diff --git a/openedx_authz/engine/config/model.conf b/openedx_authz/engine/config/model.conf index 89bb2bd8..206baca4 100644 --- a/openedx_authz/engine/config/model.conf +++ b/openedx_authz/engine/config/model.conf @@ -92,4 +92,4 @@ e = some(where (p.eft == allow)) && !some(where (p.eft == deny)) # 1. Subject must have role in scope OR global role # 2. Scope must match pattern # 3. Action must match OR inherit via action grouping -m = (g(r.sub, p.sub, r.scope) || g(r.sub, p.sub, "*")) && keyMatch(r.scope, p.scope) && (r.act == p.act || g2(p.act, r.act)) +m = ((g(r.sub, p.sub, r.scope) || g(r.sub, p.sub, "*")) && keyMatch(r.scope, p.scope) && (r.act == p.act || g2(p.act, r.act))) || custom_check(r.sub, r.act, r.scope) diff --git a/openedx_authz/engine/enforcer.py b/openedx_authz/engine/enforcer.py index f9c8a335..9d3c0d6d 100644 --- a/openedx_authz/engine/enforcer.py +++ b/openedx_authz/engine/enforcer.py @@ -23,12 +23,14 @@ from openedx_authz.engine.adapter import ExtendedAdapter from openedx_authz.engine.watcher import Watcher +from openedx_authz.engine.config.casbin_utilities import check_custom_conditions logger = logging.getLogger(__name__) adapter = ExtendedAdapter() enforcer = FastEnforcer(settings.CASBIN_MODEL, adapter, enable_log=True) enforcer.enable_auto_save(True) +enforcer.add_function("custom_check", check_custom_conditions) if Watcher: try: diff --git a/openedx_authz/settings/common.py b/openedx_authz/settings/common.py index 22feabd3..5947a54b 100644 --- a/openedx_authz/settings/common.py +++ b/openedx_authz/settings/common.py @@ -21,6 +21,9 @@ def plugin_settings(settings): if casbin_adapter_app not in settings.INSTALLED_APPS: settings.INSTALLED_APPS.append(casbin_adapter_app) + # For testing purposes, we add the test app + settings.INSTALLED_APPS.append('cms.djangoapps.course_creators') + # Add Casbin configuration settings.CASBIN_MODEL = os.path.join(ROOT_DIRECTORY, "engine", "config", "model.conf") settings.CASBIN_WATCHER_ENABLED = True