Skip to content

Commit

Permalink
Add sanitize function for redirect parameter next
Browse files Browse the repository at this point in the history
  • Loading branch information
Vladyslav Tymofeiev committed Jul 15, 2024
1 parent 2de42bd commit 631e00e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 1 deletion.
18 changes: 18 additions & 0 deletions common/djangoapps/student/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import json
import logging
import mimetypes
import re
import urllib.parse
from collections import OrderedDict
from datetime import datetime
Expand Down Expand Up @@ -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__)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
24 changes: 23 additions & 1 deletion common/djangoapps/student/tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)

0 comments on commit 631e00e

Please sign in to comment.