Merge pull request #1401 from appsembler/vladyslav/add-sanitize-next-…

…parameter-helper

Add sanitize function for redirect parameter next

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

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)