diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 42386e2b2e..709009f06b 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -329,7 +329,7 @@ def get_redirect_to(request): # get information about a user on edx.org. In any such case drop the parameter. if redirect_to: mime_type, _ = mimetypes.guess_type(redirect_to, strict=False) - if not http.is_safe_url(redirect_to, allowed_hosts={request.get_host()}): + if not http.is_safe_url(redirect_to, allowed_hosts={request.get_host()}, require_https=True): log.warning( u'Unsafe redirect parameter detected after login page: %(redirect_to)r', {"redirect_to": redirect_to} diff --git a/common/djangoapps/student/tests/test_helpers.py b/common/djangoapps/student/tests/test_helpers.py index a8aec32f0f..3a47dcebbd 100644 --- a/common/djangoapps/student/tests/test_helpers.py +++ b/common/djangoapps/student/tests/test_helpers.py @@ -66,7 +66,6 @@ class TestLoginHelper(TestCase): @ddt.data( ('/dashboard', 'testserver', '/dashboard'), - ('http://testserver/courses', 'testserver', 'http://testserver/courses'), ('https://edx.org/courses', 'edx.org', 'https://edx.org/courses'), ) @ddt.unpack diff --git a/common/djangoapps/student/views/login.py b/common/djangoapps/student/views/login.py index 2efbdde592..aea20140be 100644 --- a/common/djangoapps/student/views/login.py +++ b/common/djangoapps/student/views/login.py @@ -747,7 +747,7 @@ class LogoutView(TemplateView): """ target_url = self.request.GET.get('redirect_url') - if target_url and is_safe_url(target_url, allowed_hosts={self.request.META.get('HTTP_HOST')}): + if target_url and is_safe_url(target_url, allowed_hosts={self.request.META.get('HTTP_HOST')}, require_https=True): return target_url else: return self.default_target diff --git a/openedx/core/djangoapps/external_auth/tests/test_helper.py b/openedx/core/djangoapps/external_auth/tests/test_helper.py index b028d62dcd..6afd2926ca 100644 --- a/openedx/core/djangoapps/external_auth/tests/test_helper.py +++ b/openedx/core/djangoapps/external_auth/tests/test_helper.py @@ -19,11 +19,10 @@ class ExternalAuthHelperFnTest(TestCase): ONSITE3 = 'http://{}/my/custom/url'.format(HOST) # pylint: disable=invalid-name OFFSITE1 = 'http://www.attacker.com' # pylint: disable=invalid-name - for redirect_to in [ONSITE1, ONSITE2, ONSITE3]: + for redirect_to in [ONSITE1, ONSITE2, ONSITE3, OFFSITE1]: redir = _safe_postlogin_redirect(redirect_to, HOST) self.assertEqual(redir.status_code, 302) - self.assertEqual(redir['location'], redirect_to) - - redir2 = _safe_postlogin_redirect(OFFSITE1, HOST) - self.assertEqual(redir2.status_code, 302) - self.assertEqual("/", redir2['location']) + if redirect_to in [ONSITE3, OFFSITE1]: + self.assertEqual(redir['location'], "/") + else: + self.assertEqual(redir['location'], redirect_to) diff --git a/openedx/core/djangoapps/external_auth/views.py b/openedx/core/djangoapps/external_auth/views.py index 24829d1ab7..a57ee08af6 100644 --- a/openedx/core/djangoapps/external_auth/views.py +++ b/openedx/core/djangoapps/external_auth/views.py @@ -556,7 +556,7 @@ def _safe_postlogin_redirect(redirect_to, safehost, default_redirect='/'): @param safehost: which host is safe to redirect to @return: an HttpResponseRedirect """ - if is_safe_url(url=redirect_to, allowed_hosts={safehost}): + if is_safe_url(url=redirect_to, allowed_hosts={safehost}, require_https=True): return redirect(redirect_to) return redirect(default_redirect)