diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 39b48ece31..3d23b14e31 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -1,9 +1,11 @@ """Helpers for the student app. """ from datetime import datetime +import logging import urllib from pytz import UTC from django.core.urlresolvers import reverse, NoReverseMatch +from django.utils import http from oauth2_provider.models import ( AccessToken as dot_access_token, RefreshToken as dot_refresh_token @@ -33,6 +35,9 @@ DISABLE_UNENROLL_CERT_STATES = [ ] +log = logging.getLogger(__name__) + + def check_verify_status_by_course(user, course_enrollments): """ Determine the per-course verification statuses for a given user. @@ -239,6 +244,16 @@ def get_next_url_for_login_page(request): specified. """ redirect_to = request.GET.get('next', None) + + # if we get a redirect parameter, make sure it's safe. If it's not, drop the + # parameter. + if redirect_to and not http.is_safe_url(redirect_to): + log.error( + u'Unsafe redirect parameter detected: %(redirect_to)r', + {"redirect_to": redirect_to} + ) + redirect_to = None + if not redirect_to: try: redirect_to = reverse('dashboard') diff --git a/common/djangoapps/student/tests/test_helpers.py b/common/djangoapps/student/tests/test_helpers.py new file mode 100644 index 0000000000..687fbfd396 --- /dev/null +++ b/common/djangoapps/student/tests/test_helpers.py @@ -0,0 +1,36 @@ +""" Test Student helpers """ + +import logging + +from django.core.urlresolvers import reverse +from django.test import TestCase +from django.test.client import RequestFactory +from testfixtures import LogCapture + +from student.helpers import get_next_url_for_login_page + + +LOGGER_NAME = "student.helpers" + + +class TestLoginHelper(TestCase): + """Test login helper methods.""" + def setUp(self): + super(TestLoginHelper, self).setUp() + self.request = RequestFactory() + + def test_unsafe_next(self): + """ Test unsafe next parameter """ + unsafe_url = "https://www.amazon.com" + with LogCapture(LOGGER_NAME, level=logging.ERROR) as logger: + req = self.request.get(reverse("login") + "?next={url}".format(url=unsafe_url)) + get_next_url_for_login_page(req) + logger.check( + (LOGGER_NAME, "ERROR", u"Unsafe redirect parameter detected: u'{url}'".format(url=unsafe_url)) + ) + + def test_safe_next(self): + """ Test safe next parameter """ + req = self.request.get(reverse("login") + "?next={url}".format(url="/dashboard")) + next_page = get_next_url_for_login_page(req) + self.assertEqual(next_page, u'/dashboard')