Merge pull request #19096 from edx/robrap/ARCH-256-move-redirect-helpers

ARCH-256: Move and rename redirect helper.

common/djangoapps/student/helpers.py

@@ -15,7 +15,6 @@ from django.core.validators import ValidationError
 from django.contrib.auth import load_backend
 from django.contrib.auth.models import User
 from django.db import IntegrityError, transaction
-from django.utils import http
 from django.utils.translation import ugettext as _
 from pytz import UTC
 from six import iteritems, text_type
@@ -37,6 +36,7 @@ from openedx.core.djangoapps.certificates.api import certificates_viewable_for_c
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangoapps.theming import helpers as theming_helpers
 from openedx.core.djangoapps.theming.helpers import get_themes
+from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect
 from student.models import (
     LinkedInAddToProfileConfiguration,
     PasswordHistory,
@@ -326,7 +326,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 is_safe_redirect(request, redirect_to):
+        if not is_safe_login_or_logout_redirect(request, redirect_to):
             log.warning(
                 u'Unsafe redirect parameter detected after login page: %(redirect_to)r',
                 {"redirect_to": redirect_to}
@@ -369,19 +369,6 @@ 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

common/djangoapps/student/tests/test_helpers.py

@@ -12,7 +12,7 @@ from django.test.utils import override_settings
 from mock import patch
 from testfixtures import LogCapture
 
-from student.helpers import get_next_url_for_login_page, is_safe_redirect
+from student.helpers import get_next_url_for_login_page
 from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration_context
 
 LOGGER_NAME = "student.helpers"
@@ -78,22 +78,6 @@ class TestLoginHelper(TestCase):
         next_page = get_next_url_for_login_page(req)
         self.assertEqual(next_page, next_url)
 
-    @ddt.data(
-        ('/dashboard', 'testserver', True),
-        ('https://edx.org/courses', 'edx.org', True),
-        ('https://test.edx.org/courses', 'edx.org', True),
-        ('https://www.amazon.org', 'edx.org', False),
-        ('http://edx.org/courses', 'edx.org', False),
-        ('http:///edx.org/courses', 'edx.org', False),  # Django's is_safe_url protects against "///"
-    )
-    @ddt.unpack
-    @override_settings(LOGIN_REDIRECT_WHITELIST=['test.edx.org'])
-    def test_safe_redirect(self, url, host, expected_is_safe):
-        """ Test safe next parameter """
-        req = self.request.get(reverse("login"), HTTP_HOST=host)
-        actual_is_safe = is_safe_redirect(req, url)
-        self.assertEqual(actual_is_safe, expected_is_safe)
-
     @patch('student.helpers.third_party_auth.pipeline.get')
     @ddt.data(
         # Test requests outside the TPA pipeline - tpa_hint should be added.

openedx/core/djangoapps/user_authn/tests/test_utils.py

@@ -0,0 +1,33 @@
+""" Test User Authentication utilities """
+
+import ddt
+from django.test import TestCase
+from django.test.client import RequestFactory
+from django.test.utils import override_settings
+
+from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect
+
+
+@ddt.ddt
+class TestRedirectUtils(TestCase):
+    """Test redirect utility methods."""
+
+    def setUp(self):
+        super(TestRedirectUtils, self).setUp()
+        self.request = RequestFactory()
+
+    @ddt.data(
+        ('/dashboard', 'testserver', True),
+        ('https://edx.org/courses', 'edx.org', True),
+        ('https://test.edx.org/courses', 'edx.org', True),
+        ('https://www.amazon.org', 'edx.org', False),
+        ('http://edx.org/courses', 'edx.org', False),
+        ('http:///edx.org/courses', 'edx.org', False),  # Django's is_safe_url protects against "///"
+    )
+    @ddt.unpack
+    @override_settings(LOGIN_REDIRECT_WHITELIST=['test.edx.org'])
+    def test_safe_redirect(self, url, host, expected_is_safe):
+        """ Test safe next parameter """
+        req = self.request.get('/login', HTTP_HOST=host)
+        actual_is_safe = is_safe_login_or_logout_redirect(req, url)
+        self.assertEqual(actual_is_safe, expected_is_safe)

openedx/core/djangoapps/user_authn/utils.py

@@ -0,0 +1,18 @@
+"""
+Utility functions used during user authentication.
+"""
+from django.conf import settings
+from django.utils import http
+
+
+def is_safe_login_or_logout_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

openedx/core/djangoapps/user_authn/views/logout.py

@@ -10,7 +10,7 @@ from django.utils.http import urlencode
 from django.views.generic import TemplateView
 from provider.oauth2.models import Client
 from openedx.core.djangoapps.user_authn.cookies import delete_logged_in_cookies
-from student.helpers import is_safe_redirect
+from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect
 
 
 class LogoutView(TemplateView):
@@ -35,7 +35,7 @@ class LogoutView(TemplateView):
         """
         target_url = self.request.GET.get('redirect_url')
 
-        if target_url and is_safe_redirect(self.request, target_url):
+        if target_url and is_safe_login_or_logout_redirect(self.request, target_url):
             return target_url
         else:
             return self.default_target