diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index e46b493b7b39..a2b6f07d15ef 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -24,8 +24,10 @@ get_courses_accessible_to_user ) from common.djangoapps.course_action_state.models import CourseRerunState +from common.djangoapps.student.models.user import CourseAccessRole from common.djangoapps.student.roles import ( CourseInstructorRole, + CourseLimitedStaffRole, CourseStaffRole, GlobalStaff, OrgInstructorRole, @@ -188,6 +190,48 @@ def test_staff_course_listing(self): with self.assertNumQueries(2): list(_accessible_courses_summary_iter(self.request)) + def test_course_limited_staff_course_listing(self): + # Setup a new course + course_location = self.store.make_course_key('Org', 'CreatedCourse', 'Run') + CourseFactory.create( + org=course_location.org, + number=course_location.course, + run=course_location.run + ) + course = CourseOverviewFactory.create(id=course_location, org=course_location.org) + + # Add the user as a course_limited_staff on the course + CourseLimitedStaffRole(course.id).add_users(self.user) + self.assertTrue(CourseLimitedStaffRole(course.id).has_user(self.user)) + + # Fetch accessible courses list & verify their count + courses_list_by_staff, __ = get_courses_accessible_to_user(self.request) + + # Limited Course Staff should not be able to list courses in Studio + assert len(list(courses_list_by_staff)) == 0 + + def test_org_limited_staff_course_listing(self): + + # Setup a new course + course_location = self.store.make_course_key('Org', 'CreatedCourse', 'Run') + CourseFactory.create( + org=course_location.org, + number=course_location.course, + run=course_location.run + ) + course = CourseOverviewFactory.create(id=course_location, org=course_location.org) + + # Add a user as course_limited_staff on the org + # This is not possible using the course roles classes but is possible via Django admin so we + # insert a row into the model directly to test that scenario. + CourseAccessRole.objects.create(user=self.user, org=course_location.org, role=CourseLimitedStaffRole.ROLE) + + # Fetch accessible courses list & verify their count + courses_list_by_staff, __ = get_courses_accessible_to_user(self.request) + + # Limited Course Staff should not be able to list courses in Studio + assert len(list(courses_list_by_staff)) == 0 + def test_get_course_list_with_invalid_course_location(self): """ Test getting courses with invalid course location (course deleted from modulestore). diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 81ad1eb6ddde..deeb4a4ee2fe 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -53,6 +53,7 @@ GlobalStaff, UserBasedRole, OrgStaffRole, + strict_role_checking, ) from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json from common.djangoapps.util.string_utils import _has_non_ascii_characters @@ -533,7 +534,9 @@ def filter_ccx(course_access): return not isinstance(course_access.course_id, CCXLocator) instructor_courses = UserBasedRole(request.user, CourseInstructorRole.ROLE).courses_with_role() - staff_courses = UserBasedRole(request.user, CourseStaffRole.ROLE).courses_with_role() + with strict_role_checking(): + staff_courses = UserBasedRole(request.user, CourseStaffRole.ROLE).courses_with_role() + all_courses = list(filter(filter_ccx, instructor_courses | staff_courses)) courses_list = [] course_keys = {} diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index e199142fe377..047f0174a062 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -24,6 +24,7 @@ OrgInstructorRole, OrgLibraryUserRole, OrgStaffRole, + strict_role_checking, ) # Studio permissions: @@ -115,8 +116,9 @@ def get_user_permissions(user, course_key, org=None, service_variant=None): return STUDIO_NO_PERMISSIONS # Staff have all permissions except EDIT_ROLES: - if OrgStaffRole(org=org).has_user(user) or (course_key and user_has_role(user, CourseStaffRole(course_key))): - return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT + with strict_role_checking(): + if OrgStaffRole(org=org).has_user(user) or (course_key and user_has_role(user, CourseStaffRole(course_key))): + return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT # Otherwise, for libraries, users can view only: if course_key and isinstance(course_key, LibraryLocator): diff --git a/common/djangoapps/student/tests/test_authz.py b/common/djangoapps/student/tests/test_authz.py index c0b88e6318b5..70636e04b68a 100644 --- a/common/djangoapps/student/tests/test_authz.py +++ b/common/djangoapps/student/tests/test_authz.py @@ -11,6 +11,7 @@ from django.test import TestCase, override_settings from opaque_keys.edx.locator import CourseLocator +from common.djangoapps.student.models.user import CourseAccessRole from common.djangoapps.student.auth import ( add_users, has_studio_read_access, @@ -305,6 +306,23 @@ def test_limited_staff_no_studio_access_cms(self): assert not has_studio_read_access(self.limited_staff, self.course_key) assert not has_studio_write_access(self.limited_staff, self.course_key) + @override_settings(SERVICE_VARIANT='cms') + def test_limited_org_staff_no_studio_access_cms(self): + """ + Verifies that course limited staff have no read and no write access when SERVICE_VARIANT is not 'lms'. + """ + # Add a user as course_limited_staff on the org + # This is not possible using the course roles classes but is possible via Django admin so we + # insert a row into the model directly to test that scenario. + CourseAccessRole.objects.create( + user=self.limited_staff, + org=self.course_key.org, + role=CourseLimitedStaffRole.ROLE, + ) + + assert not has_studio_read_access(self.limited_staff, self.course_key) + assert not has_studio_write_access(self.limited_staff, self.course_key) + class CourseOrgGroupTest(TestCase): """