diff --git a/lms/djangoapps/course_home_api/course_metadata/views.py b/lms/djangoapps/course_home_api/course_metadata/views.py index e534d03ebbf7..476a7eb2866c 100644 --- a/lms/djangoapps/course_home_api/course_metadata/views.py +++ b/lms/djangoapps/course_home_api/course_metadata/views.py @@ -15,6 +15,7 @@ from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.course_api.api import course_detail from lms.djangoapps.course_goals.models import UserActivity +from lms.djangoapps.course_home_api import permissions from lms.djangoapps.course_home_api.course_metadata.serializers import CourseHomeMetadataSerializer from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs @@ -76,9 +77,8 @@ def get(self, request, *args, **kwargs): course_key = CourseKey.from_string(course_key_string) original_user_is_global_staff = self.request.user.is_staff original_user_is_staff = has_access(request.user, 'staff', course_key).has_access - + user_can_masquarade = request.user.has_perm(permissions.CAN_MASQUARADE_LEARNER_PROGRESS, course_key) course = course_detail(request, request.user.username, course_key) - # We must compute course load access *before* setting up masquerading, # else course staff (who are not enrolled) will not be able view # their course from the perspective of a learner. @@ -86,7 +86,7 @@ def get(self, request, *args, **kwargs): course, request.user, 'load', - check_if_enrolled=True, + check_if_enrolled=(not user_can_masquarade), check_if_authenticated=True, apply_enterprise_checks=True, ) @@ -94,7 +94,7 @@ def get(self, request, *args, **kwargs): _, request.user = setup_masquerade( request, course_key, - staff_access=original_user_is_staff, + staff_access=user_can_masquarade, reset_masquerade_data=True, ) diff --git a/lms/djangoapps/course_home_api/permissions.py b/lms/djangoapps/course_home_api/permissions.py new file mode 100644 index 000000000000..e17d5dd3c0a1 --- /dev/null +++ b/lms/djangoapps/course_home_api/permissions.py @@ -0,0 +1,10 @@ +""" +Permissions for the course home apis and associated actions +""" +from bridgekeeper import perms +from lms.djangoapps.courseware.rules import HasAccessRule + + +CAN_MASQUARADE_LEARNER_PROGRESS = 'course_home_api.can_masquarade_progress' + +perms[CAN_MASQUARADE_LEARNER_PROGRESS] = HasAccessRule('staff') diff --git a/lms/djangoapps/course_home_api/progress/views.py b/lms/djangoapps/course_home_api/progress/views.py index dc0ea63525f7..6f2d53339e73 100644 --- a/lms/djangoapps/course_home_api/progress/views.py +++ b/lms/djangoapps/course_home_api/progress/views.py @@ -14,6 +14,7 @@ from xmodule.modulestore.django import modulestore from common.djangoapps.student.models import CourseEnrollment +from lms.djangoapps.course_home_api import permissions from lms.djangoapps.course_home_api.progress.serializers import ProgressTabSerializer from lms.djangoapps.course_home_api.toggles import course_home_mfe_progress_tab_is_active from lms.djangoapps.courseware.access import has_access, has_ccx_coach_role @@ -186,8 +187,9 @@ def get(self, request, *args, **kwargs): monitoring_utils.set_custom_attribute('user_id', request.user.id) monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff) is_staff = bool(has_access(request.user, 'staff', course_key)) + can_masquarade = request.user.has_perm(permissions.CAN_MASQUARADE_LEARNER_PROGRESS, course_key) - student = self._get_student_user(request, course_key, student_id, is_staff) + student = self._get_student_user(request, course_key, student_id, can_masquarade) username = get_enterprise_learner_generic_name(request) or student.username course = get_course_with_access(student, 'load', course_key, check_if_enrolled=False) @@ -196,7 +198,7 @@ def get(self, request, *args, **kwargs): enrollment = CourseEnrollment.get_enrollment(student, course_key) enrollment_mode = getattr(enrollment, 'mode', None) - if not (enrollment and enrollment.is_active) and not is_staff: + if not (enrollment and enrollment.is_active) and not can_masquarade: return Response('User not enrolled.', status=401) # The block structure is used for both the course_grade and has_scheduled content fields diff --git a/lms/djangoapps/course_home_api/tests/test_permissions.py b/lms/djangoapps/course_home_api/tests/test_permissions.py new file mode 100644 index 000000000000..5edac2f2d3d3 --- /dev/null +++ b/lms/djangoapps/course_home_api/tests/test_permissions.py @@ -0,0 +1,70 @@ +""" +Tests for permissions defined in courseware.rules +""" +import ddt + +from common.djangoapps.student.roles import OrgStaffRole, CourseStaffRole +from common.djangoapps.student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order +from lms.djangoapps.course_home_api.permissions import CAN_MASQUARADE_LEARNER_PROGRESS + + +@ddt.ddt +class PermissionTests(ModuleStoreTestCase): + """ + Tests for permissions defined in courseware.rules + """ + def setUp(self): + super().setUp() + self.user = UserFactory() + self.course = CourseFactory(org='org') + self.another_course = CourseFactory(org='org') + + def tearDown(self): + super().tearDown() + self.user.delete() + + @ddt.data( + ( + True, None, None, True, + "Global staff users should have masquerade access", + ), + ( + False, None, None, False, + "Non-staff users shouldn't have masquerade access", + ), + ( + False, 'another_org', None, False, + "User with staff access on another org shouldn't have masquerade access", + ), + ( + False, 'org', None, True, + "User with org-wide staff access should have masquerade access", + ), + ( + False, None, 'another_course', False, + "User with staff access on another course shouldn't have masquerade access", + ), + ( + False, None, 'course', True, + "User with staff access on the course should have masquerade access", + ), + ) + @ddt.unpack + def test_can_masquerade_return_value(self, is_staff, org_role, course_role, expected_permission, description): + """ + Test that only authorized users have masquerade access + """ + self.user.is_staff = is_staff + self.user.save() + assert self.user.is_staff == is_staff + + if org_role: + OrgStaffRole(org_role).add_users(self.user) + + if course_role: + CourseStaffRole(getattr(self, course_role).id).add_users(self.user) + + has_perm = self.user.has_perm(CAN_MASQUARADE_LEARNER_PROGRESS, self.course.id) + assert has_perm == expected_permission, description