diff --git a/openedx_authz/constants/permissions.py b/openedx_authz/constants/permissions.py index ed1ba42c..f9f64a59 100644 --- a/openedx_authz/constants/permissions.py +++ b/openedx_authz/constants/permissions.py @@ -168,6 +168,11 @@ effect="allow", ) +COURSES_VIEW_ADVANCED_SETTINGS = PermissionData( + action=ActionData(external_key=f"{COURSES_NAMESPACE}.view_advanced_settings"), + effect="allow", +) + COURSES_MANAGE_CERTIFICATES = PermissionData( action=ActionData(external_key=f"{COURSES_NAMESPACE}.manage_certificates"), effect="allow", diff --git a/openedx_authz/constants/roles.py b/openedx_authz/constants/roles.py index 99badb03..40322cf8 100644 --- a/openedx_authz/constants/roles.py +++ b/openedx_authz/constants/roles.py @@ -69,6 +69,7 @@ permissions.COURSES_VIEW_CHECKLISTS, permissions.COURSES_VIEW_COURSE_TEAM, permissions.COURSES_VIEW_SCHEDULE_AND_DETAILS, + permissions.COURSES_VIEW_ADVANCED_SETTINGS, ] COURSE_AUDITOR = RoleData(external_key="course_auditor", permissions=COURSE_AUDITOR_PERMISSIONS) @@ -82,6 +83,7 @@ permissions.COURSES_VIEW_CHECKLISTS, permissions.COURSES_VIEW_COURSE_TEAM, permissions.COURSES_VIEW_SCHEDULE_AND_DETAILS, + permissions.COURSES_VIEW_ADVANCED_SETTINGS, permissions.COURSES_EDIT_COURSE_CONTENT, permissions.COURSES_MANAGE_LIBRARY_UPDATES, permissions.COURSES_MANAGE_COURSE_UPDATES, @@ -92,6 +94,7 @@ permissions.COURSES_MANAGE_GROUP_CONFIGURATIONS, permissions.COURSES_EDIT_DETAILS, permissions.COURSES_MANAGE_TAGS, + permissions.COURSES_MANAGE_ADVANCED_SETTINGS, ] COURSE_EDITOR = RoleData(external_key="course_editor", permissions=COURSE_EDITOR_PERMISSIONS) @@ -106,6 +109,7 @@ permissions.COURSES_VIEW_CHECKLISTS, permissions.COURSES_VIEW_COURSE_TEAM, permissions.COURSES_VIEW_SCHEDULE_AND_DETAILS, + permissions.COURSES_VIEW_ADVANCED_SETTINGS, permissions.COURSES_EDIT_COURSE_CONTENT, permissions.COURSES_MANAGE_LIBRARY_UPDATES, permissions.COURSES_MANAGE_COURSE_UPDATES, @@ -140,6 +144,7 @@ permissions.COURSES_VIEW_CHECKLISTS, permissions.COURSES_VIEW_COURSE_TEAM, permissions.COURSES_VIEW_SCHEDULE_AND_DETAILS, + permissions.COURSES_VIEW_ADVANCED_SETTINGS, permissions.COURSES_EDIT_COURSE_CONTENT, permissions.COURSES_MANAGE_LIBRARY_UPDATES, permissions.COURSES_MANAGE_COURSE_UPDATES, diff --git a/openedx_authz/engine/config/authz.policy b/openedx_authz/engine/config/authz.policy index 4dfb3d1b..6b1299c0 100644 --- a/openedx_authz/engine/config/authz.policy +++ b/openedx_authz/engine/config/authz.policy @@ -81,6 +81,7 @@ p, role^course_auditor, act^courses.view_grading_settings, course-v1^*, allow p, role^course_auditor, act^courses.view_checklists, course-v1^*, allow p, role^course_auditor, act^courses.view_course_team, course-v1^*, allow p, role^course_auditor, act^courses.view_schedule_and_details, course-v1^*, allow +p, role^course_auditor, act^courses.view_advanced_settings, course-v1^*, allow # Course Editor Role Policies p, role^course_editor, act^courses.view_course, course-v1^*, allow @@ -91,6 +92,7 @@ p, role^course_editor, act^courses.view_grading_settings, course-v1^*, allow p, role^course_editor, act^courses.view_checklists, course-v1^*, allow p, role^course_editor, act^courses.view_course_team, course-v1^*, allow p, role^course_editor, act^courses.view_schedule_and_details, course-v1^*, allow +p, role^course_editor, act^courses.view_advanced_settings, course-v1^*, allow p, role^course_editor, act^courses.edit_course_content, course-v1^*, allow p, role^course_editor, act^courses.manage_library_updates, course-v1^*, allow p, role^course_editor, act^courses.manage_course_updates, course-v1^*, allow @@ -101,6 +103,7 @@ p, role^course_editor, act^courses.edit_grading_settings, course-v1^*, allow p, role^course_editor, act^courses.manage_group_configurations, course-v1^*, allow p, role^course_editor, act^courses.edit_details, course-v1^*, allow p, role^course_editor, act^courses.manage_tags, course-v1^*, allow +p, role^course_editor, act^courses.manage_advanced_settings, course-v1^*, allow # Course Staff Role Policies p, role^course_staff, act^courses.legacy_staff_role_permissions, course-v1^*, allow @@ -112,6 +115,7 @@ p, role^course_staff, act^courses.view_grading_settings, course-v1^*, allow p, role^course_staff, act^courses.view_checklists, course-v1^*, allow p, role^course_staff, act^courses.view_course_team, course-v1^*, allow p, role^course_staff, act^courses.view_schedule_and_details, course-v1^*, allow +p, role^course_staff, act^courses.view_advanced_settings, course-v1^*, allow p, role^course_staff, act^courses.edit_course_content, course-v1^*, allow p, role^course_staff, act^courses.manage_library_updates, course-v1^*, allow p, role^course_staff, act^courses.manage_course_updates, course-v1^*, allow @@ -142,6 +146,7 @@ p, role^course_admin, act^courses.view_grading_settings, course-v1^*, allow p, role^course_admin, act^courses.view_checklists, course-v1^*, allow p, role^course_admin, act^courses.view_course_team, course-v1^*, allow p, role^course_admin, act^courses.view_schedule_and_details, course-v1^*, allow +p, role^course_admin, act^courses.view_advanced_settings, course-v1^*, allow p, role^course_admin, act^courses.edit_course_content, course-v1^*, allow p, role^course_admin, act^courses.manage_library_updates, course-v1^*, allow p, role^course_admin, act^courses.manage_course_updates, course-v1^*, allow diff --git a/openedx_authz/tests/test_enforcement.py b/openedx_authz/tests/test_enforcement.py index a79d3487..8b65cfef 100644 --- a/openedx_authz/tests/test_enforcement.py +++ b/openedx_authz/tests/test_enforcement.py @@ -26,6 +26,8 @@ from openedx_authz.constants import roles from openedx_authz.constants.permissions import ( COURSES_CREATE_FILES, + COURSES_MANAGE_ADVANCED_SETTINGS, + COURSES_VIEW_ADVANCED_SETTINGS, COURSES_VIEW_COURSE, MANAGE_LIBRARY_TEAM, VIEW_LIBRARY, @@ -897,3 +899,84 @@ def test_is_admin_or_superuser_check( """ request = {"subject": subject, "action": action, "scope": scope, "expected_result": expected_result} self._test_enforcement(self.POLICY, request) + + +@ddt +class AdvancedSettingsPermissionsTests(CasbinEnforcementTestCase): + """ + Tests for advanced settings permissions across all course roles. + + Verifies the two-tier access model: + - course_auditor: VIEW only (read-only access) + - course_editor, course_admin, course_staff: both VIEW and MANAGE (full access) + """ + + COURSE = "course-v1:TestOrg+TestCourse+2024_T1" + + POLICIES = [ + make_policy( + roles.COURSE_AUDITOR.external_key, + COURSES_VIEW_ADVANCED_SETTINGS.identifier, + CourseOverviewData.NAMESPACE, + ), + make_policy( + roles.COURSE_EDITOR.external_key, + COURSES_VIEW_ADVANCED_SETTINGS.identifier, + CourseOverviewData.NAMESPACE, + ), + make_policy( + roles.COURSE_EDITOR.external_key, + COURSES_MANAGE_ADVANCED_SETTINGS.identifier, + CourseOverviewData.NAMESPACE, + ), + make_policy( + roles.COURSE_ADMIN.external_key, + COURSES_VIEW_ADVANCED_SETTINGS.identifier, + CourseOverviewData.NAMESPACE, + ), + make_policy( + roles.COURSE_ADMIN.external_key, + COURSES_MANAGE_ADVANCED_SETTINGS.identifier, + CourseOverviewData.NAMESPACE, + ), + make_policy( + roles.COURSE_STAFF.external_key, + COURSES_VIEW_ADVANCED_SETTINGS.identifier, + CourseOverviewData.NAMESPACE, + ), + make_policy( + roles.COURSE_STAFF.external_key, + COURSES_MANAGE_ADVANCED_SETTINGS.identifier, + CourseOverviewData.NAMESPACE, + ), + ] + + ASSIGNMENTS = [ + make_course_assignment("auditor", roles.COURSE_AUDITOR.external_key, COURSE), + make_course_assignment("editor", roles.COURSE_EDITOR.external_key, COURSE), + make_course_assignment("admin", roles.COURSE_ADMIN.external_key, COURSE), + make_course_assignment("staff", roles.COURSE_STAFF.external_key, COURSE), + ] + + CASES = [ + # course_auditor: view allowed, manage denied + make_course_case("auditor", COURSES_VIEW_ADVANCED_SETTINGS.identifier, COURSE, True), + make_course_case("auditor", COURSES_MANAGE_ADVANCED_SETTINGS.identifier, COURSE, False), + # course_editor: both view and manage allowed + make_course_case("editor", COURSES_VIEW_ADVANCED_SETTINGS.identifier, COURSE, True), + make_course_case("editor", COURSES_MANAGE_ADVANCED_SETTINGS.identifier, COURSE, True), + # course_admin: both view and manage allowed + make_course_case("admin", COURSES_VIEW_ADVANCED_SETTINGS.identifier, COURSE, True), + make_course_case("admin", COURSES_MANAGE_ADVANCED_SETTINGS.identifier, COURSE, True), + # course_staff: both view and manage allowed + make_course_case("staff", COURSES_VIEW_ADVANCED_SETTINGS.identifier, COURSE, True), + make_course_case("staff", COURSES_MANAGE_ADVANCED_SETTINGS.identifier, COURSE, True), + # unassigned user: both denied + make_course_case("other", COURSES_VIEW_ADVANCED_SETTINGS.identifier, COURSE, False), + make_course_case("other", COURSES_MANAGE_ADVANCED_SETTINGS.identifier, COURSE, False), + ] + + @data(*CASES) + def test_advanced_settings_enforcement(self, request: AuthRequest): + """Test that advanced settings permissions are enforced correctly per role.""" + self._test_enforcement(self.POLICIES + self.ASSIGNMENTS, request) diff --git a/openedx_authz/tests/test_engine_utils.py b/openedx_authz/tests/test_engine_utils.py index 5315c97f..8034519f 100644 --- a/openedx_authz/tests/test_engine_utils.py +++ b/openedx_authz/tests/test_engine_utils.py @@ -76,10 +76,10 @@ def test_migrate_all_file_policies_to_database(self): Expected Result: - All policies from the file are loaded into the database - - The file contains 116 regular policies (p rules) + - The file contains 121 regular policies (p rules) - Policy content matches expected file content """ - expected_policy_count = 116 + expected_policy_count = 121 migrate_policy_between_enforcers(self.source_enforcer, self.target_enforcer) self.target_enforcer.load_policy() @@ -208,7 +208,7 @@ def test_migrate_complete_file_contents(self): """Test that all policy types from the file are migrated correctly. Expected Result: - - All regular policies (p) are migrated (116 rules) + - All regular policies (p) are migrated (121 rules) - No role assignments (g) - these come from database - All action inheritance rules (g2) are migrated (10 rules) """ @@ -216,8 +216,8 @@ def test_migrate_complete_file_contents(self): self.assertEqual( len(self.target_enforcer.get_policy()), - 116, - "Should have 116 regular policies from file", + 121, + "Should have 121 regular policies from file", ) self.assertEqual( len(self.target_enforcer.get_grouping_policy()), @@ -250,8 +250,8 @@ def test_migrate_partial_duplicates(self): target_policies = self.target_enforcer.get_policy() self.assertEqual( len(target_policies), - 116, - "Should have 116 policies total, with no duplicates", + 121, + "Should have 121 policies total, with no duplicates", ) duplicates = CasbinRule.objects.values("v0", "v1", "v2").annotate(total=Count("*")).filter(total__gt=1) @@ -346,7 +346,7 @@ def test_migrate_preserves_existing_db_policies(self): migrate_policy_between_enforcers(self.source_enforcer, self.target_enforcer) target_policies = self.target_enforcer.get_policy() - self.assertEqual(len(target_policies), 117, "Should have 116 file policies + 1 custom policy") + self.assertEqual(len(target_policies), 122, "Should have 121 file policies + 1 custom policy") self.assertIn(custom_policy, target_policies, "Custom database policy should be preserved") def test_migrate_preserves_user_role_assignments_in_db(self): @@ -382,4 +382,4 @@ def test_migrate_preserves_user_role_assignments_in_db(self): ) target_policies = self.target_enforcer.get_policy() - self.assertEqual(len(target_policies), 116, "All 116 policies from file should be loaded") + self.assertEqual(len(target_policies), 121, "All 121 policies from file should be loaded")