Skip to content

Commit 461788b

Browse files
feanilDeimerM
authored andcommitted
fix: CourseLimitedStaffRole should not be able to access studio.
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.
1 parent 314cd1e commit 461788b

File tree

4 files changed

+70
-3
lines changed

4 files changed

+70
-3
lines changed

cms/djangoapps/contentstore/tests/test_course_listing.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@
2424
get_courses_accessible_to_user
2525
)
2626
from common.djangoapps.course_action_state.models import CourseRerunState
27+
from common.djangoapps.student.models.user import CourseAccessRole
2728
from common.djangoapps.student.roles import (
2829
CourseInstructorRole,
30+
CourseLimitedStaffRole,
2931
CourseStaffRole,
3032
GlobalStaff,
3133
OrgInstructorRole,
@@ -188,6 +190,48 @@ def test_staff_course_listing(self):
188190
with self.assertNumQueries(2):
189191
list(_accessible_courses_summary_iter(self.request))
190192

193+
def test_course_limited_staff_course_listing(self):
194+
# Setup a new course
195+
course_location = self.store.make_course_key('Org', 'CreatedCourse', 'Run')
196+
CourseFactory.create(
197+
org=course_location.org,
198+
number=course_location.course,
199+
run=course_location.run
200+
)
201+
course = CourseOverviewFactory.create(id=course_location, org=course_location.org)
202+
203+
# Add the user as a course_limited_staff on the course
204+
CourseLimitedStaffRole(course.id).add_users(self.user)
205+
self.assertTrue(CourseLimitedStaffRole(course.id).has_user(self.user))
206+
207+
# Fetch accessible courses list & verify their count
208+
courses_list_by_staff, __ = get_courses_accessible_to_user(self.request)
209+
210+
# Limited Course Staff should not be able to list courses in Studio
211+
assert len(list(courses_list_by_staff)) == 0
212+
213+
def test_org_limited_staff_course_listing(self):
214+
215+
# Setup a new course
216+
course_location = self.store.make_course_key('Org', 'CreatedCourse', 'Run')
217+
CourseFactory.create(
218+
org=course_location.org,
219+
number=course_location.course,
220+
run=course_location.run
221+
)
222+
course = CourseOverviewFactory.create(id=course_location, org=course_location.org)
223+
224+
# Add a user as course_limited_staff on the org
225+
# This is not possible using the course roles classes but is possible via Django admin so we
226+
# insert a row into the model directly to test that scenario.
227+
CourseAccessRole.objects.create(user=self.user, org=course_location.org, role=CourseLimitedStaffRole.ROLE)
228+
229+
# Fetch accessible courses list & verify their count
230+
courses_list_by_staff, __ = get_courses_accessible_to_user(self.request)
231+
232+
# Limited Course Staff should not be able to list courses in Studio
233+
assert len(list(courses_list_by_staff)) == 0
234+
191235
def test_get_course_list_with_invalid_course_location(self):
192236
"""
193237
Test getting courses with invalid course location (course deleted from modulestore).

cms/djangoapps/contentstore/views/course.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
GlobalStaff,
5454
UserBasedRole,
5555
OrgStaffRole,
56+
strict_role_checking,
5657
)
5758
from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json
5859
from common.djangoapps.util.string_utils import _has_non_ascii_characters
@@ -533,7 +534,9 @@ def filter_ccx(course_access):
533534
return not isinstance(course_access.course_id, CCXLocator)
534535

535536
instructor_courses = UserBasedRole(request.user, CourseInstructorRole.ROLE).courses_with_role()
536-
staff_courses = UserBasedRole(request.user, CourseStaffRole.ROLE).courses_with_role()
537+
with strict_role_checking():
538+
staff_courses = UserBasedRole(request.user, CourseStaffRole.ROLE).courses_with_role()
539+
537540
all_courses = list(filter(filter_ccx, instructor_courses | staff_courses))
538541
courses_list = []
539542
course_keys = {}

common/djangoapps/student/auth.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
OrgInstructorRole,
2525
OrgLibraryUserRole,
2626
OrgStaffRole,
27+
strict_role_checking,
2728
)
2829

2930
# Studio permissions:
@@ -115,8 +116,9 @@ def get_user_permissions(user, course_key, org=None, service_variant=None):
115116
return STUDIO_NO_PERMISSIONS
116117

117118
# Staff have all permissions except EDIT_ROLES:
118-
if OrgStaffRole(org=org).has_user(user) or (course_key and user_has_role(user, CourseStaffRole(course_key))):
119-
return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT
119+
with strict_role_checking():
120+
if OrgStaffRole(org=org).has_user(user) or (course_key and user_has_role(user, CourseStaffRole(course_key))):
121+
return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT
120122

121123
# Otherwise, for libraries, users can view only:
122124
if course_key and isinstance(course_key, LibraryLocator):

common/djangoapps/student/tests/test_authz.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from django.test import TestCase, override_settings
1212
from opaque_keys.edx.locator import CourseLocator
1313

14+
from common.djangoapps.student.models.user import CourseAccessRole
1415
from common.djangoapps.student.auth import (
1516
add_users,
1617
has_studio_read_access,
@@ -305,6 +306,23 @@ def test_limited_staff_no_studio_access_cms(self):
305306
assert not has_studio_read_access(self.limited_staff, self.course_key)
306307
assert not has_studio_write_access(self.limited_staff, self.course_key)
307308

309+
@override_settings(SERVICE_VARIANT='cms')
310+
def test_limited_org_staff_no_studio_access_cms(self):
311+
"""
312+
Verifies that course limited staff have no read and no write access when SERVICE_VARIANT is not 'lms'.
313+
"""
314+
# Add a user as course_limited_staff on the org
315+
# This is not possible using the course roles classes but is possible via Django admin so we
316+
# insert a row into the model directly to test that scenario.
317+
CourseAccessRole.objects.create(
318+
user=self.limited_staff,
319+
org=self.course_key.org,
320+
role=CourseLimitedStaffRole.ROLE,
321+
)
322+
323+
assert not has_studio_read_access(self.limited_staff, self.course_key)
324+
assert not has_studio_write_access(self.limited_staff, self.course_key)
325+
308326

309327
class CourseOrgGroupTest(TestCase):
310328
"""

0 commit comments

Comments
 (0)