From 23b9cfd76c49e88d1a73c4824120d061880df319 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Fri, 21 Sep 2018 15:45:11 -0400 Subject: [PATCH] Add whitelist for login redirect. Use LOGIN_REDIRECT_WHITELIST to provide a whitelist of additional domains to which login will now redirect. ARCH-238 --- common/djangoapps/student/helpers.py | 19 ++++++++++++++++--- .../djangoapps/student/tests/test_helpers.py | 3 +++ lms/envs/aws.py | 3 +++ lms/envs/common.py | 2 ++ lms/envs/devstack.py | 2 ++ 5 files changed, 26 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 0a7e62898b..d9cbc86f11 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -273,7 +273,7 @@ def get_next_url_for_login_page(request): If THIRD_PARTY_AUTH_HINT is set, then `tpa_hint=` is added as a query parameter. """ - redirect_to = get_redirect_to(request) + redirect_to = _get_redirect_to(request) if not redirect_to: try: redirect_to = reverse('dashboard') @@ -308,7 +308,7 @@ def get_next_url_for_login_page(request): return redirect_to -def get_redirect_to(request): +def _get_redirect_to(request): """ Determine the redirect url and return if safe :argument @@ -325,7 +325,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()}, require_https=True): + if not _is_safe_redirect(request, redirect_to): log.warning( u'Unsafe redirect parameter detected after login page: %(redirect_to)r', {"redirect_to": redirect_to} @@ -368,6 +368,19 @@ def get_redirect_to(request): return redirect_to +def _is_safe_redirect(request, redirect_to): + """ + Determine if the given redirect URL/path is safe for redirection. + """ + request_host = request.get_host() # e.g. 'courses.edx.org' + + login_redirect_whitelist = set(getattr(settings, 'LOGIN_REDIRECT_WHITELIST', [])) + login_redirect_whitelist.add(request_host) + + is_safe_url = http.is_safe_url(redirect_to, allowed_hosts=login_redirect_whitelist, require_https=True) + return is_safe_url + + def generate_activation_email_context(user, registration): """ Constructs a dictionary for use in activation email contexts diff --git a/common/djangoapps/student/tests/test_helpers.py b/common/djangoapps/student/tests/test_helpers.py index 3a47dcebbd..ff6b8c216f 100644 --- a/common/djangoapps/student/tests/test_helpers.py +++ b/common/djangoapps/student/tests/test_helpers.py @@ -67,8 +67,11 @@ class TestLoginHelper(TestCase): @ddt.data( ('/dashboard', 'testserver', '/dashboard'), ('https://edx.org/courses', 'edx.org', 'https://edx.org/courses'), + ('https://test.edx.org/courses', 'edx.org', 'https://test.edx.org/courses'), + ('https://test2.edx.org/courses', 'edx.org', 'https://test2.edx.org/courses'), ) @ddt.unpack + @override_settings(LOGIN_REDIRECT_WHITELIST=['test.edx.org', 'test2.edx.org']) def test_safe_next(self, url, host, expected_url): """ Test safe next parameter """ req = self.request.get(reverse("login") + "?next={url}".format(url=url), HTTP_HOST=host) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 53d7a5e433..75a5d10893 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -419,6 +419,9 @@ NOTIFICATION_EMAIL_EDX_LOGO = ENV_TOKENS.get('NOTIFICATION_EMAIL_EDX_LOGO', NOTI # by end users. CSRF_COOKIE_SECURE = ENV_TOKENS.get('CSRF_COOKIE_SECURE', False) +# Whitelist of domains to which the login/logout pages will redirect. +LOGIN_REDIRECT_WHITELIST = ENV_TOKENS.get('LOGIN_REDIRECT_WHITELIST', LOGIN_REDIRECT_WHITELIST) + ############# CORS headers for cross-domain requests ################# if FEATURES.get('ENABLE_CORS_HEADERS') or FEATURES.get('ENABLE_CROSS_DOMAIN_CSRF_COOKIE'): diff --git a/lms/envs/common.py b/lms/envs/common.py index 33f026c16e..40c3f822c1 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2509,6 +2509,8 @@ if FEATURES.get('ENABLE_CORS_HEADERS'): # to simulate cross-domain requests. XDOMAIN_PROXY_CACHE_TIMEOUT = 60 * 15 +LOGIN_REDIRECT_WHITELIST = [] + ###################### Registration ################################## # For each of the fields, give one of the following values: diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index c715f40d87..63661f00f9 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -228,6 +228,8 @@ CORS_ALLOW_CREDENTIALS = True CORS_ORIGIN_WHITELIST = () CORS_ORIGIN_ALLOW_ALL = True +LOGIN_REDIRECT_WHITELIST = [] + ###################### JWTs ###################### JWT_AUTH.update({ 'JWT_ISSUER': OAUTH_OIDC_ISSUER,