Skip to content

Conversation

@robrap
Copy link

@robrap robrap commented Dec 17, 2025

Description

We previously fixed this when the CourseLimitedStaffRole was applied to a course but did not handle the case where the role is applied to a user for a whole org. The underlying issue is that the CourseLimitedStaffRole is a subclass of the CourseStaffRole and much of the system assumes that subclesses are for giving more access not less access.

To prevent that from happening for the case of the CourseLimitedStaffRole, when we do CourseStaffRole access checks, we use the strict_role_checking context manager to ensure that we're not accidentally granting the limited_staff role too much access.

Note

This change was cherry-picked from openedx/edx-platform.

We previously fixed this when the CourseLimitedStaffRole was applied to
a course but did not handle the case where the role is applied to a user
for a whole org.  The underlying issue is that the CourseLimitedStaffRole
is a subclass of the CourseStaffRole and much of the system assumes that
subclesses are for giving more access not less access.

To prevent that from happening for the case of the CourseLimitedStaffRole,
when we do CourseStaffRole access checks, we use the strict_role_checking
context manager to ensure that we're not accidentally granting the
limited_staff role too much access.
Copilot AI review requested due to automatic review settings December 17, 2025 19:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a security issue where users with the CourseLimitedStaffRole assigned at the organization level could gain unintended Studio/CMS access. The underlying problem is that CourseLimitedStaffRole is a subclass of CourseStaffRole, and the role inheritance system assumes subclasses have more permissions, not fewer. This fix uses the strict_role_checking() context manager to temporarily disable role inheritance when checking CourseStaffRole permissions in Studio contexts, ensuring that users with CourseLimitedStaffRole are properly restricted.

Key changes:

  • Wrapped CourseStaffRole permission checks with strict_role_checking() in get_user_permissions() and _accessible_courses_list_from_groups() to prevent role inheritance from granting unintended access
  • Added comprehensive tests for both course-level and org-level CourseLimitedStaffRole assignments to verify Studio access is properly denied

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
common/djangoapps/student/auth.py Adds strict_role_checking() wrapper around CourseStaffRole permission check in get_user_permissions() to prevent CourseLimitedStaffRole from inheriting studio permissions
cms/djangoapps/contentstore/views/course.py Applies strict_role_checking() when retrieving courses accessible to staff users in _accessible_courses_list_from_groups() to prevent limited staff from listing courses
common/djangoapps/student/tests/test_authz.py Adds test case for org-level CourseLimitedStaffRole to verify no Studio access is granted in CMS context
cms/djangoapps/contentstore/tests/test_course_listing.py Adds tests for both course-level and org-level CourseLimitedStaffRole to ensure users cannot list courses in Studio

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

number=course_location.course,
run=course_location.run
)
course = CourseOverviewFactory.create(id=course_location, org=course_location.org)
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

Variable course is not used.

Suggested change
course = CourseOverviewFactory.create(id=course_location, org=course_location.org)
CourseOverviewFactory.create(id=course_location, org=course_location.org)

Copilot uses AI. Check for mistakes.
@timmc-edx timmc-edx enabled auto-merge (squash) December 17, 2025 19:20
@timmc-edx timmc-edx merged commit f987e8e into release-ulmo Dec 17, 2025
69 checks passed
@timmc-edx timmc-edx deleted the robrap/add-role-fix branch December 17, 2025 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants