From eb91cee05c61c51e90852562529f0f69499d7c70 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Mon, 27 Apr 2026 17:54:33 -0500 Subject: [PATCH 1/2] feat: add advanced settings view permission for course roles --- openedx_authz/constants/permissions.py | 5 +++++ openedx_authz/constants/roles.py | 3 +++ openedx_authz/engine/config/authz.policy | 2 ++ 3 files changed, 10 insertions(+) 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..64b2510d 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) diff --git a/openedx_authz/engine/config/authz.policy b/openedx_authz/engine/config/authz.policy index 4dfb3d1b..aaa96885 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 @@ -101,6 +102,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 From 76d9854869914b1ec474e207d8a9223184d7afb1 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Thu, 30 Apr 2026 10:29:13 -0500 Subject: [PATCH 2/2] feat: add advanced settings permissions for course_editor and course_auditor roles --- openedx_authz/engine/config/authz.policy | 1 + openedx_authz/tests/test_enforcement.py | 55 ++++++++++++++++++++++++ openedx_authz/tests/test_engine_utils.py | 14 +++--- 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/openedx_authz/engine/config/authz.policy b/openedx_authz/engine/config/authz.policy index aaa96885..286b86bc 100644 --- a/openedx_authz/engine/config/authz.policy +++ b/openedx_authz/engine/config/authz.policy @@ -92,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 diff --git a/openedx_authz/tests/test_enforcement.py b/openedx_authz/tests/test_enforcement.py index a79d3487..a37a5833 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,56 @@ 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 for course_auditor and course_editor roles. + + Verifies the two-tier access model: + - course_auditor: VIEW only (read-only access) + - course_editor: 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, + ), + ] + + ASSIGNMENTS = [ + make_course_assignment("auditor", roles.COURSE_AUDITOR.external_key, COURSE), + make_course_assignment("editor", roles.COURSE_EDITOR.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), + # 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..c6ba8701 100644 --- a/openedx_authz/tests/test_engine_utils.py +++ b/openedx_authz/tests/test_engine_utils.py @@ -79,7 +79,7 @@ def test_migrate_all_file_policies_to_database(self): - The file contains 116 regular policies (p rules) - Policy content matches expected file content """ - expected_policy_count = 116 + expected_policy_count = 119 migrate_policy_between_enforcers(self.source_enforcer, self.target_enforcer) self.target_enforcer.load_policy() @@ -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", + 119, + "Should have 119 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", + 119, + "Should have 119 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), 120, "Should have 119 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), 119, "All 119 policies from file should be loaded")