Skip to content

Commit 76d9854

Browse files
committed
feat: add advanced settings permissions for course_editor and course_auditor roles
1 parent eb91cee commit 76d9854

3 files changed

Lines changed: 63 additions & 7 deletions

File tree

openedx_authz/engine/config/authz.policy

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ p, role^course_editor, act^courses.view_grading_settings, course-v1^*, allow
9292
p, role^course_editor, act^courses.view_checklists, course-v1^*, allow
9393
p, role^course_editor, act^courses.view_course_team, course-v1^*, allow
9494
p, role^course_editor, act^courses.view_schedule_and_details, course-v1^*, allow
95+
p, role^course_editor, act^courses.view_advanced_settings, course-v1^*, allow
9596
p, role^course_editor, act^courses.edit_course_content, course-v1^*, allow
9697
p, role^course_editor, act^courses.manage_library_updates, course-v1^*, allow
9798
p, role^course_editor, act^courses.manage_course_updates, course-v1^*, allow

openedx_authz/tests/test_enforcement.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
from openedx_authz.constants import roles
2727
from openedx_authz.constants.permissions import (
2828
COURSES_CREATE_FILES,
29+
COURSES_MANAGE_ADVANCED_SETTINGS,
30+
COURSES_VIEW_ADVANCED_SETTINGS,
2931
COURSES_VIEW_COURSE,
3032
MANAGE_LIBRARY_TEAM,
3133
VIEW_LIBRARY,
@@ -897,3 +899,56 @@ def test_is_admin_or_superuser_check(
897899
"""
898900
request = {"subject": subject, "action": action, "scope": scope, "expected_result": expected_result}
899901
self._test_enforcement(self.POLICY, request)
902+
903+
904+
@ddt
905+
class AdvancedSettingsPermissionsTests(CasbinEnforcementTestCase):
906+
"""
907+
Tests for advanced settings permissions for course_auditor and course_editor roles.
908+
909+
Verifies the two-tier access model:
910+
- course_auditor: VIEW only (read-only access)
911+
- course_editor: both VIEW and MANAGE (full access)
912+
"""
913+
914+
COURSE = "course-v1:TestOrg+TestCourse+2024_T1"
915+
916+
POLICIES = [
917+
make_policy(
918+
roles.COURSE_AUDITOR.external_key,
919+
COURSES_VIEW_ADVANCED_SETTINGS.identifier,
920+
CourseOverviewData.NAMESPACE,
921+
),
922+
make_policy(
923+
roles.COURSE_EDITOR.external_key,
924+
COURSES_VIEW_ADVANCED_SETTINGS.identifier,
925+
CourseOverviewData.NAMESPACE,
926+
),
927+
make_policy(
928+
roles.COURSE_EDITOR.external_key,
929+
COURSES_MANAGE_ADVANCED_SETTINGS.identifier,
930+
CourseOverviewData.NAMESPACE,
931+
),
932+
]
933+
934+
ASSIGNMENTS = [
935+
make_course_assignment("auditor", roles.COURSE_AUDITOR.external_key, COURSE),
936+
make_course_assignment("editor", roles.COURSE_EDITOR.external_key, COURSE),
937+
]
938+
939+
CASES = [
940+
# course_auditor: view allowed, manage denied
941+
make_course_case("auditor", COURSES_VIEW_ADVANCED_SETTINGS.identifier, COURSE, True),
942+
make_course_case("auditor", COURSES_MANAGE_ADVANCED_SETTINGS.identifier, COURSE, False),
943+
# course_editor: both view and manage allowed
944+
make_course_case("editor", COURSES_VIEW_ADVANCED_SETTINGS.identifier, COURSE, True),
945+
make_course_case("editor", COURSES_MANAGE_ADVANCED_SETTINGS.identifier, COURSE, True),
946+
# unassigned user: both denied
947+
make_course_case("other", COURSES_VIEW_ADVANCED_SETTINGS.identifier, COURSE, False),
948+
make_course_case("other", COURSES_MANAGE_ADVANCED_SETTINGS.identifier, COURSE, False),
949+
]
950+
951+
@data(*CASES)
952+
def test_advanced_settings_enforcement(self, request: AuthRequest):
953+
"""Test that advanced settings permissions are enforced correctly per role."""
954+
self._test_enforcement(self.POLICIES + self.ASSIGNMENTS, request)

openedx_authz/tests/test_engine_utils.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def test_migrate_all_file_policies_to_database(self):
7979
- The file contains 116 regular policies (p rules)
8080
- Policy content matches expected file content
8181
"""
82-
expected_policy_count = 116
82+
expected_policy_count = 119
8383

8484
migrate_policy_between_enforcers(self.source_enforcer, self.target_enforcer)
8585
self.target_enforcer.load_policy()
@@ -216,8 +216,8 @@ def test_migrate_complete_file_contents(self):
216216

217217
self.assertEqual(
218218
len(self.target_enforcer.get_policy()),
219-
116,
220-
"Should have 116 regular policies from file",
219+
119,
220+
"Should have 119 regular policies from file",
221221
)
222222
self.assertEqual(
223223
len(self.target_enforcer.get_grouping_policy()),
@@ -250,8 +250,8 @@ def test_migrate_partial_duplicates(self):
250250
target_policies = self.target_enforcer.get_policy()
251251
self.assertEqual(
252252
len(target_policies),
253-
116,
254-
"Should have 116 policies total, with no duplicates",
253+
119,
254+
"Should have 119 policies total, with no duplicates",
255255
)
256256

257257
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):
346346
migrate_policy_between_enforcers(self.source_enforcer, self.target_enforcer)
347347

348348
target_policies = self.target_enforcer.get_policy()
349-
self.assertEqual(len(target_policies), 117, "Should have 116 file policies + 1 custom policy")
349+
self.assertEqual(len(target_policies), 120, "Should have 119 file policies + 1 custom policy")
350350
self.assertIn(custom_policy, target_policies, "Custom database policy should be preserved")
351351

352352
def test_migrate_preserves_user_role_assignments_in_db(self):
@@ -382,4 +382,4 @@ def test_migrate_preserves_user_role_assignments_in_db(self):
382382
)
383383

384384
target_policies = self.target_enforcer.get_policy()
385-
self.assertEqual(len(target_policies), 116, "All 116 policies from file should be loaded")
385+
self.assertEqual(len(target_policies), 119, "All 119 policies from file should be loaded")

0 commit comments

Comments
 (0)