From 7432cbded20e5dceb31c723e7391316ff297ceb1 Mon Sep 17 00:00:00 2001 From: AliAkbar Date: Tue, 25 Jan 2022 02:25:35 +0500 Subject: [PATCH] fix: escape vulnerable enrollment mode --- lms/djangoapps/support/tests/test_views.py | 60 ++++++++++++++++++--- lms/djangoapps/support/views/enrollments.py | 10 ++-- 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/support/tests/test_views.py b/lms/djangoapps/support/tests/test_views.py index 04dae1a384..deab43c567 100644 --- a/lms/djangoapps/support/tests/test_views.py +++ b/lms/djangoapps/support/tests/test_views.py @@ -2,7 +2,6 @@ Tests for support views. """ - import itertools import json import re @@ -27,6 +26,7 @@ from edx_proctoring.tests.utils import ProctoredExamTestCase from opaque_keys.edx.locator import BlockUsageLocator from organizations.tests.factories import OrganizationFactory from pytz import UTC +from rest_framework import status from social_django.models import UserSocialAuth from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -247,6 +247,7 @@ class SupportViewCertificatesTests(SupportViewTestCase): """ Tests for the certificates support view. """ + def setUp(self): """Make the user support staff. """ super().setUp() @@ -433,6 +434,27 @@ class SupportViewEnrollmentsTests(SharedModuleStoreTestCase, SupportViewTestCase assert manual_enrollment.enrolled_email == 'test2@example.com' assert manual_enrollment.state_transition == UNENROLLED_TO_ENROLLED + @disable_signal(signals, 'post_save') + @ddt.data('username', 'email') + def test_create_new_enrollment_invalid_mode(self, search_string_type): + """ + Assert that a new enrollment is not created with a vulnerable/invalid enrollment mode. + """ + test_user = UserFactory.create(username='newStudent', email='test2@example.com', password='test') + assert ManualEnrollmentAudit.get_manual_enrollment_by_email(test_user.email) is None + url = reverse( + 'support:enrollment_list', + kwargs={'username_or_email': getattr(test_user, search_string_type)} + ) + response = self.client.post(url, data={ + 'course_id': str(self.course.id), + 'mode': '', + 'reason': 'Financial Assistance' + }) + test_key_error = b'<script>alert("xss")</script> is not a valid mode for org' + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert test_key_error in response.content + @disable_signal(signals, 'post_save') @ddt.data('username', 'email') def test_create_existing_enrollment(self, search_string_type): @@ -473,6 +495,27 @@ class SupportViewEnrollmentsTests(SharedModuleStoreTestCase, SupportViewTestCase assert ManualEnrollmentAudit.get_manual_enrollment_by_email(self.student.email) is not None self.assert_enrollment(CourseMode.VERIFIED) + @disable_signal(signals, 'post_save') + @ddt.data('username', 'email') + def test_change_enrollment_invalid_old_mode(self, search_string_type): + """ + Assert changing mode fails for an enrollment having a vulnerable/invalid old mode. + """ + assert ManualEnrollmentAudit.get_manual_enrollment_by_email(self.student.email) is None + url = reverse( + 'support:enrollment_list', + kwargs={'username_or_email': getattr(self.student, search_string_type)} + ) + response = self.client.patch(url, data={ + 'course_id': str(self.course.id), + 'old_mode': '', + 'new_mode': CourseMode.VERIFIED, + 'reason': 'Financial Assistance' + }, content_type='application/json') + test_key_error = b'is not enrolled with mode <script>alert("xss")</script>' + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert test_key_error in response.content + @disable_signal(signals, 'post_save') @ddt.data('username', 'email') @patch("common.djangoapps.entitlements.models.get_course_uuid_for_course") @@ -711,8 +754,8 @@ class SupportViewLinkProgramEnrollmentsTests(SupportViewTestCase): assert render_call_dict['errors'] == [error] @ddt.data( - '0001,learner-01\n0002,learner-02', # normal - '0001,learner-01,apple,orange\n0002,learner-02,purple', # extra fields + '0001,learner-01\n0002,learner-02', # normal + '0001,learner-01,apple,orange\n0002,learner-02,purple', # extra fields '\t0001 , \t learner-01 \n 0002 , learner-02 ', # whitespace ) @patch('lms.djangoapps.support.views.utils.link_program_enrollments') @@ -902,7 +945,8 @@ class ProgramEnrollmentsInspectorViewTests(SupportViewTestCase): is_active=True ) - program_course_enrollment = ProgramCourseEnrollmentFactory.create( # lint-amnesty, pylint: disable=unused-variable + program_course_enrollment = ProgramCourseEnrollmentFactory.create( + # lint-amnesty, pylint: disable=unused-variable program_enrollment=program_enrollment, course_key=course_id, course_enrollment=course_enrollment, @@ -1254,7 +1298,8 @@ class ProgramEnrollmentsInspectorAPIViewTests(SupportViewTestCase): is_active=True ) - program_course_enrollment = ProgramCourseEnrollmentFactory.create( # lint-amnesty, pylint: disable=unused-variable + program_course_enrollment = ProgramCourseEnrollmentFactory.create( + # lint-amnesty, pylint: disable=unused-variable program_enrollment=program_enrollment, course_key=course_id, course_enrollment=course_enrollment, @@ -1498,6 +1543,7 @@ class FeatureBasedEnrollmentSupportApiViewTests(SupportViewTestCase): """ Test suite for FBE Support API view. """ + def setUp(self): super().setUp() SupportStaffRole().add_users(self.user) @@ -1624,8 +1670,8 @@ class LinkProgramEnrollmentSupportAPIViewTests(SupportViewTestCase): assert response_data['errors'] == [error] @ddt.data( - '0001,learner-01\n0002,learner-02', # normal - '0001,learner-01,apple,orange\n0002,learner-02,purple', # extra fields + '0001,learner-01\n0002,learner-02', # normal + '0001,learner-01,apple,orange\n0002,learner-02,purple', # extra fields '\t0001 , \t learner-01 \n 0002 , learner-02 ', # whitespace ) @patch('lms.djangoapps.support.views.utils.link_program_enrollments') diff --git a/lms/djangoapps/support/views/enrollments.py b/lms/djangoapps/support/views/enrollments.py index 4b1d68dab9..fba5cb801f 100644 --- a/lms/djangoapps/support/views/enrollments.py +++ b/lms/djangoapps/support/views/enrollments.py @@ -4,6 +4,7 @@ Support tool for changing course enrollments. from collections import defaultdict +import markupsafe from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.db import transaction from django.db.models import Q @@ -162,7 +163,7 @@ class EnrollmentSupportListView(GenericAPIView): ] if mode not in enrollment_modes: return HttpResponseBadRequest( - f'{str(mode)} is not a valid mode for {str(course_id)}. ' + f'{markupsafe.escape(mode)} is not a valid mode for {str(course_id)}. ' f'Possible valid modes are {str(enrollment_modes)}' ) @@ -192,10 +193,9 @@ class EnrollmentSupportListView(GenericAPIView): reason = request.data['reason'] enrollment = CourseEnrollment.objects.get(user=user, course_id=course_key) if enrollment.mode != old_mode: - return HttpResponseBadRequest('User {username} is not enrolled with mode {old_mode}.'.format( - username=user.username, - old_mode=old_mode - )) + return HttpResponseBadRequest( + f'User {user.username} is not enrolled with mode {markupsafe.escape(old_mode)}.' + ) except KeyError as err: return HttpResponseBadRequest(f'The field {str(err)} is required.') except InvalidKeyError: