fix: respect masqueraded learner permissions when computing subsection show_grades in Progress Tab API (#38025)
This PR fixes grade-visibility behavior in the Progress Tab API when staff users are masquerading as learners. Previously, subsection show_grades was computed using the real requester’s staff access, which could expose grades that the masqueraded learner should not see. Now, show_grades respects the masqueraded user’s permissions so the API response matches the learner view. Co-authored-by: Peter Pinch <pdpinch@mit.edu>
This commit is contained in:
@@ -117,6 +117,49 @@ class ProgressTabTestViews(BaseCourseHomeTests):
|
|||||||
self.update_masquerade(username=verified_user.username)
|
self.update_masquerade(username=verified_user.username)
|
||||||
assert self.client.get(self.url).data.get('enrollment_mode') == 'verified'
|
assert self.client.get(self.url).data.get('enrollment_mode') == 'verified'
|
||||||
|
|
||||||
|
def test_masquerade_uses_masqueraded_permissions_for_show_grades(self):
|
||||||
|
"""
|
||||||
|
Test that when a staff user is masquerading as a verified learner,
|
||||||
|
the grade visibility for subsections with show_correctness='past_due' is
|
||||||
|
determined based on the verified learner's permissions, not the staff user's
|
||||||
|
permissions.
|
||||||
|
"""
|
||||||
|
subsection_name = 'Masquerade grade visibility subsection'
|
||||||
|
chapter = BlockFactory(parent=self.course, category='chapter')
|
||||||
|
subsection = BlockFactory(
|
||||||
|
parent=chapter,
|
||||||
|
category='sequential',
|
||||||
|
display_name=subsection_name,
|
||||||
|
graded=True,
|
||||||
|
due=now() + timedelta(days=30),
|
||||||
|
show_correctness='past_due',
|
||||||
|
)
|
||||||
|
vertical = BlockFactory(parent=subsection, category='vertical', graded=True)
|
||||||
|
BlockFactory(parent=vertical, category='problem', graded=True)
|
||||||
|
|
||||||
|
verified_user = UserFactory(is_staff=False)
|
||||||
|
CourseEnrollment.enroll(verified_user, self.course.id, CourseMode.VERIFIED)
|
||||||
|
|
||||||
|
self.switch_to_staff() # needed for masquerade
|
||||||
|
|
||||||
|
def get_subsection_show_grades(response):
|
||||||
|
for chapter in response.data['section_scores']:
|
||||||
|
for subsection in chapter['subsections']:
|
||||||
|
if subsection['display_name'] == subsection_name:
|
||||||
|
return subsection['show_grades']
|
||||||
|
assert False, f'Subsection {subsection_name} not found in section_scores'
|
||||||
|
|
||||||
|
# Staff can see grades even when show_correctness is `past_due` and the due date has not passed.
|
||||||
|
response = self.client.get(self.url)
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert get_subsection_show_grades(response) is True
|
||||||
|
|
||||||
|
# When masquerading, grade visibility should follow the masqueraded learner permissions.
|
||||||
|
self.update_masquerade(username=verified_user.username)
|
||||||
|
response = self.client.get(self.url)
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert get_subsection_show_grades(response) is False
|
||||||
|
|
||||||
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
|
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
|
||||||
def test_has_scheduled_content_data(self):
|
def test_has_scheduled_content_data(self):
|
||||||
CourseEnrollment.enroll(self.user, self.course.id)
|
CourseEnrollment.enroll(self.user, self.course.id)
|
||||||
|
|||||||
@@ -191,7 +191,7 @@ class ProgressTabView(RetrieveAPIView):
|
|||||||
visible_chapters.append({**chapter, "sections": filtered_sections})
|
visible_chapters.append({**chapter, "sections": filtered_sections})
|
||||||
return visible_chapters
|
return visible_chapters
|
||||||
|
|
||||||
def get(self, request, *args, **kwargs):
|
def get(self, request, *args, **kwargs): # pylint: disable=too-many-statements
|
||||||
course_key_string = kwargs.get('course_key_string')
|
course_key_string = kwargs.get('course_key_string')
|
||||||
course_key = CourseKey.from_string(course_key_string)
|
course_key = CourseKey.from_string(course_key_string)
|
||||||
student_id = kwargs.get('student_id')
|
student_id = kwargs.get('student_id')
|
||||||
@@ -203,9 +203,10 @@ class ProgressTabView(RetrieveAPIView):
|
|||||||
monitoring_utils.set_custom_attribute('course_id', course_key_string)
|
monitoring_utils.set_custom_attribute('course_id', course_key_string)
|
||||||
monitoring_utils.set_custom_attribute('user_id', request.user.id)
|
monitoring_utils.set_custom_attribute('user_id', request.user.id)
|
||||||
monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff)
|
monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff)
|
||||||
is_staff = bool(has_access(request.user, 'staff', course_key))
|
requester_has_staff_access = bool(has_access(request.user, 'staff', course_key))
|
||||||
|
|
||||||
student = self._get_student_user(request, course_key, student_id, is_staff)
|
student = self._get_student_user(request, course_key, student_id, requester_has_staff_access)
|
||||||
|
learner_has_staff_access = bool(has_access(student, 'staff', course_key))
|
||||||
username = get_enterprise_learner_generic_name(request) or student.username
|
username = get_enterprise_learner_generic_name(request) or student.username
|
||||||
|
|
||||||
course = get_course_or_403(student, 'load', course_key, check_if_enrolled=False)
|
course = get_course_or_403(student, 'load', course_key, check_if_enrolled=False)
|
||||||
@@ -214,7 +215,7 @@ class ProgressTabView(RetrieveAPIView):
|
|||||||
enrollment = CourseEnrollment.get_enrollment(student, course_key)
|
enrollment = CourseEnrollment.get_enrollment(student, course_key)
|
||||||
enrollment_mode = getattr(enrollment, 'mode', None)
|
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 requester_has_staff_access:
|
||||||
return Response('User not enrolled.', status=401)
|
return Response('User not enrolled.', status=401)
|
||||||
|
|
||||||
# The block structure is used for both the course_grade and has_scheduled content fields
|
# The block structure is used for both the course_grade and has_scheduled content fields
|
||||||
@@ -223,7 +224,7 @@ class ProgressTabView(RetrieveAPIView):
|
|||||||
course_grade = CourseGradeFactory().read(student, collected_block_structure=collected_block_structure)
|
course_grade = CourseGradeFactory().read(student, collected_block_structure=collected_block_structure)
|
||||||
|
|
||||||
# recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not)
|
# recalculate course grade from visible grades (stored grade was calculated over all grades, visible or not)
|
||||||
course_grade.update(visible_grades_only=True, has_staff_access=is_staff)
|
course_grade.update(visible_grades_only=True, has_staff_access=learner_has_staff_access)
|
||||||
|
|
||||||
# Get has_scheduled_content data
|
# Get has_scheduled_content data
|
||||||
transformers = BlockStructureTransformers()
|
transformers = BlockStructureTransformers()
|
||||||
@@ -265,7 +266,7 @@ class ProgressTabView(RetrieveAPIView):
|
|||||||
assignment_type_grade_summary = aggregate_assignment_type_grade_summary(
|
assignment_type_grade_summary = aggregate_assignment_type_grade_summary(
|
||||||
course_grade,
|
course_grade,
|
||||||
grading_policy,
|
grading_policy,
|
||||||
has_staff_access=is_staff,
|
has_staff_access=learner_has_staff_access,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Filter out section scores to only have those that are visible to the user
|
# Filter out section scores to only have those that are visible to the user
|
||||||
@@ -291,7 +292,7 @@ class ProgressTabView(RetrieveAPIView):
|
|||||||
'final_grades': assignment_type_grade_summary["final_grades"],
|
'final_grades': assignment_type_grade_summary["final_grades"],
|
||||||
}
|
}
|
||||||
context = self.get_serializer_context()
|
context = self.get_serializer_context()
|
||||||
context['staff_access'] = is_staff
|
context['staff_access'] = learner_has_staff_access
|
||||||
context['course_blocks'] = course_blocks
|
context['course_blocks'] = course_blocks
|
||||||
context['course_key'] = course_key
|
context['course_key'] = course_key
|
||||||
# course_overview and enrollment will be used by VerifiedModeSerializer
|
# course_overview and enrollment will be used by VerifiedModeSerializer
|
||||||
|
|||||||
Reference in New Issue
Block a user