Skip to content

Commit

Permalink
fix: security issue limited staff have edit access through some APIs …
Browse files Browse the repository at this point in the history
…in CMS
  • Loading branch information
GlugovGrGlib authored and ormsbee committed Jun 17, 2024
1 parent b3df1dd commit 50097c2
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
13 changes: 9 additions & 4 deletions common/djangoapps/student/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,19 @@ def get_user_permissions(user, course_key, org=None):
return all_perms
# HACK: Limited Staff should not have studio read access. However, since many LMS views depend on the
# `has_course_author_access` check and `course_author_access_required` decorator, we have to allow write access
# until the permissions become more granular. For example, there could be STUDIO_VIEW_COHORTS and
# STUDIO_EDIT_COHORTS specifically for the cohorts endpoint, which is used to display the "Cohorts" tab of the
# Instructor Dashboard.
# by returning STUDIO_EDIT_CONTENT, if the request is made from LMS, until the permissions become more granular.
# For example, there could be STUDIO_VIEW_COHORTS and STUDIO_EDIT_COHORTS specifically for the cohorts endpoint,
# which is used to display the "Cohorts" tab of the Instructor Dashboard. If the request is made from the CMS,
# then STUDIO_NO_PERMISSIONS is returned instead.
# The permissions matrix from the RBAC project (https://github.com/openedx/platform-roadmap/issues/246) shows that
# the LMS and Studio permissions will be separated as a part of this project. Once this is done (and this code is
# not removed during its implementation), we can replace the Limited Staff permissions with more granular ones.
if course_key and user_has_role(user, CourseLimitedStaffRole(course_key)):
return STUDIO_EDIT_CONTENT
if settings.SERVICE_VARIANT == 'lms':
return STUDIO_EDIT_CONTENT
else:
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
Expand Down
17 changes: 14 additions & 3 deletions common/djangoapps/student/tests/test_authz.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from ccx_keys.locator import CCXLocator
from django.contrib.auth.models import AnonymousUser
from django.core.exceptions import PermissionDenied
from django.test import TestCase
from django.test import TestCase, override_settings
from opaque_keys.edx.locator import CourseLocator

from common.djangoapps.student.auth import (
Expand Down Expand Up @@ -285,15 +285,26 @@ def test_remove_user_from_course_group_permission_denied(self):
with pytest.raises(PermissionDenied):
remove_users(self.staff, CourseStaffRole(self.course_key), another_staff)

def test_limited_staff_no_studio_read_access(self):
@override_settings(SERVICE_VARIANT='lms')
def test_limited_staff_no_studio_read_access_lms(self):
"""
Verifies that course limited staff have no read, but have write access.
Verifies that course limited staff have no read, but have write access when SERVICE_VARIANT is 'lms'.
"""
add_users(self.global_admin, CourseLimitedStaffRole(self.course_key), self.limited_staff)

assert not has_studio_read_access(self.limited_staff, self.course_key)
assert has_studio_write_access(self.limited_staff, self.course_key)

@override_settings(SERVICE_VARIANT='cms')
def test_limited_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_users(self.global_admin, CourseLimitedStaffRole(self.course_key), self.limited_staff)

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

0 comments on commit 50097c2

Please sign in to comment.