From 631e00e77279e73a20fd494c05442a17c5fdd9bf Mon Sep 17 00:00:00 2001 From: Vladyslav Tymofeiev <“vladyslavty@softwareplanetgroup.com”> Date: Tue, 2 Jul 2024 18:56:10 +0300 Subject: [PATCH] Add sanitize function for redirect parameter next --- common/djangoapps/student/helpers.py | 18 ++++++++++++++ .../djangoapps/student/tests/test_helpers.py | 24 ++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 3c47510b5af1..4c2fcf7efd6c 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -6,6 +6,7 @@ import json import logging import mimetypes +import re import urllib.parse from collections import OrderedDict from datetime import datetime @@ -65,6 +66,8 @@ EMAIL_EXISTS_MSG_FMT = _("An account with the Email '{email}' already exists.") USERNAME_EXISTS_MSG_FMT = _("An account with the Public Username '{username}' already exists.") +COURSE_URL_PATTERN = re.compile(r'courses/course-v1:[^/]+/course') + log = logging.getLogger(__name__) @@ -301,6 +304,7 @@ def _get_redirect_to(request_host, request_headers, request_params, request_is_h redirect url if safe else None """ redirect_to = request_params.get('next') + redirect_to = sanitize_next_parameter(redirect_to) header_accept = request_headers.get('HTTP_ACCEPT', '') accepts_text_html = any( mime_type in header_accept @@ -700,3 +704,17 @@ def get_resume_urls_for_enrollments(user, enrollments): url_to_block = '' resume_course_urls[enrollment.course_id] = url_to_block return resume_course_urls + + +def sanitize_next_parameter(next_param): + """ + Check the next parameter pattern and update the + symbol to its ASCII equivalent. + """ + if not next_param: + return next_param + + if COURSE_URL_PATTERN.match(next_param): + sanitized_next_parameter = re.sub(r'\+', '%2B', next_param) + return sanitized_next_parameter + + return next_param diff --git a/common/djangoapps/student/tests/test_helpers.py b/common/djangoapps/student/tests/test_helpers.py index 655d9b763b36..8231503c8a55 100644 --- a/common/djangoapps/student/tests/test_helpers.py +++ b/common/djangoapps/student/tests/test_helpers.py @@ -14,7 +14,7 @@ from testfixtures import LogCapture from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration_context -from student.helpers import get_next_url_for_login_page +from student.helpers import get_next_url_for_login_page, sanitize_next_parameter LOGGER_NAME = "student.helpers" @@ -156,3 +156,25 @@ def test_custom_tahoe_site_redirect_lms(self): 'LOGIN_REDIRECT_URL': '' # Falsy or empty URLs should not be used }): assert '/dashboard' == get_next_url_for_login_page(request), 'Falsy url should default to dashboard' + + def test_sanitize_next_param(self): + # Valid URL with plus - change the plus symbol to ASCII code + next_param = 'courses/course-v1:abc-sandbox+ACC-PTF+C/course' + expected_result = 'courses/course-v1:abc-sandbox%2BACC-PTF%2BC/course' + self.assertEqual(sanitize_next_parameter(next_param), expected_result) + + # Valid URL without plus - keep the next_param as it is + next_param = 'courses/course-v1:abc-sandbox/course' + self.assertEqual(sanitize_next_parameter(next_param), next_param) + + # Empty string - keep the next_param as it is + next_param = '' + self.assertEqual(sanitize_next_parameter(next_param), next_param) + + # None input - keep the next_param as it is + next_param = None + self.assertEqual(sanitize_next_parameter(next_param), next_param) + + # Invalid pattern - keep the next_param as it is + next_param = 'some/other/path' + self.assertEqual(sanitize_next_parameter(next_param), next_param)