Merge pull request #801 from open-craft/viadanna/teak/course-limited-staff-fix

fix: CourseLimitedStaffRole should not be able to access studio.

Author: Kelketek

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).

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 = {}

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):

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):
     """