Skip to content

feat: add advanced settings view permission for course roles #272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bra-i-am wants to merge 4 commits into from
Closed

feat: add advanced settings view permission for course roles #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
67fa329
feat: add advanced settings view permission for course roles
bra-i-am Apr 27, 2026
c75398d
feat: add advanced settings permissions for course_editor and course_…
bra-i-am Apr 30, 2026
713b35a
refactor: address feedback
bra-i-am May 13, 2026
61bc401
feat: update version to 1.16.0 and document new advanced settings per…
bra-i-am May 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ Change Log
Unreleased
**********

1.16.0 - 2026-05-14
*******************

Added
=====

* Add ``COURSES_VIEW_ADVANCED_SETTINGS`` permission and assign it to ``course_auditor``, ``course_editor``, ``course_staff``, and ``course_admin`` roles to enable read-only access to Advanced Settings.
* Assign existing ``COURSES_MANAGE_ADVANCED_SETTINGS`` permission to the ``course_editor`` role to grant full read/write access to Advanced Settings.
Copy link
Copy Markdown
Contributor

@MaferMazu MaferMazu May 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the approval because I thought you hadn't added the manage_advanced_settings. Details: #288

Is there a place where we say the editor has advanced settings permission? If not, can we remove that permission for the editor role?


1.15.0 - 2026-04-30
*******************

Expand Down
2 changes: 1 addition & 1 deletion openedx_authz/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

import os

__version__ = "1.15.0"
__version__ = "1.16.0"

ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__))
5 changes: 5 additions & 0 deletions openedx_authz/constants/permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 5 additions & 0 deletions openedx_authz/constants/roles.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -82,6 +83,7 @@
permissions.COURSES_VIEW_CHECKLISTS,
permissions.COURSES_VIEW_COURSE_TEAM,
permissions.COURSES_VIEW_SCHEDULE_AND_DETAILS,
permissions.COURSES_VIEW_ADVANCED_SETTINGS,
Comment thread
MaferMazu marked this conversation as resolved.
permissions.COURSES_EDIT_COURSE_CONTENT,
permissions.COURSES_MANAGE_LIBRARY_UPDATES,
permissions.COURSES_MANAGE_COURSE_UPDATES,
Expand All @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions openedx_authz/engine/config/authz.policy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Comment thread
MaferMazu marked this conversation as resolved.
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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
83 changes: 83 additions & 0 deletions openedx_authz/tests/test_enforcement.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Comment thread
MaferMazu marked this conversation as resolved.
"""
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)
18 changes: 9 additions & 9 deletions openedx_authz/tests/test_engine_utils.py
View file
Open in desktop
Comment thread
MaferMazu marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -208,16 +208,16 @@ 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)
"""
migrate_policy_between_enforcers(self.source_enforcer, self.target_enforcer)

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()),
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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")