Merge pull request #29897 from openedx/private_to_public_120ed72

Mergeback PR from private to public.

common/djangoapps/third_party_auth/pipeline.py

@@ -71,7 +71,7 @@ import six
 import social_django
 from django.conf import settings
 from django.contrib.auth.models import User  # lint-amnesty, pylint: disable=imported-auth-user
-from django.contrib.auth import logout
+from django.contrib.auth import logout, REDIRECT_FIELD_NAME
 from django.core.mail.message import EmailMessage
 from django.http import HttpResponseBadRequest
 from django.shortcuts import redirect
@@ -90,6 +90,7 @@ from openedx.core.djangoapps.user_api import accounts
 from openedx.core.djangoapps.user_api.accounts.utils import username_suffix_generator
 from openedx.core.djangoapps.user_authn import cookies as user_authn_cookies
 from openedx.core.djangoapps.user_authn.toggles import is_require_third_party_auth_enabled
+from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect
 from common.djangoapps.third_party_auth.utils import (
     get_associated_user_by_email_response,
     get_user_from_email,
@@ -1016,3 +1017,27 @@ def get_username(strategy, details, backend, user=None, *args, **kwargs):  # lin
     else:
         final_username = storage.user.get_username(user)
     return {'username': final_username}
+
+
+def ensure_redirect_url_is_safe(strategy, *args, **kwargs):
+    """
+    Ensure that the redirect url is save if a user logs in or registers by
+    directly hitting the TPA url i.e /auth/login/backend_name?next=<redirect_to>
+
+    Check it against the LOGIN_REDIRECT_WHITELIST. If it is not safe then
+    redirect to SOCIAL_AUTH_LOGIN_REDIRECT_URL (defaults to /dashboard)
+    """
+    redirect_to = strategy.session_get(REDIRECT_FIELD_NAME, None)
+    request = strategy.request
+
+    if redirect_to and request:
+        is_safe = is_safe_login_or_logout_redirect(
+            redirect_to=redirect_to,
+            request_host=request.get_host(),
+            dot_client_id=None,
+            require_https=request.is_secure(),
+        )
+
+        if not is_safe:
+            safe_redirect_url = getattr(settings, 'SOCIAL_AUTH_LOGIN_REDIRECT_URL', '/dashboard')
+            strategy.session_set(REDIRECT_FIELD_NAME, safe_redirect_url)

common/djangoapps/third_party_auth/settings.py

@@ -67,6 +67,7 @@ def apply_settings(django_settings):
         'common.djangoapps.third_party_auth.pipeline.set_id_verification_status',
         'common.djangoapps.third_party_auth.pipeline.set_logged_in_cookies',
         'common.djangoapps.third_party_auth.pipeline.login_analytics',
+        'common.djangoapps.third_party_auth.pipeline.ensure_redirect_url_is_safe',
     ]
 
     # Add enterprise pipeline elements if the enterprise app is installed

common/djangoapps/third_party_auth/tests/test_pipeline_integration.py

@@ -8,7 +8,7 @@ import ddt
 import pytest
 import pytz
 from django import test
-from django.contrib.auth import models
+from django.contrib.auth import models, REDIRECT_FIELD_NAME
 from django.core import mail
 from social_django import models as social_models
 
@@ -655,3 +655,52 @@ class ParseQueryParamsPipelineTestCase(TestCase):
 
         with self.assertRaises(pipeline.AuthEntryError):
             pipeline.parse_query_params(self.strategy, self.response)
+
+
+class EnsureRedirectUrlIsSafeTestCase(TestCase):
+    """Tests to ensure that the redirect url is safe and user can redirect to it."""
+
+    def setUp(self):
+        super().setUp()
+
+        self.strategy = mock.MagicMock()
+        self.request = self.strategy.request
+
+        self.request.get_host.return_value = 'localhost:18000'
+        self.request.is_secure.return_value = True
+
+        self.strategy.session_get = self._session_get
+        self.strategy.session_set = self._session_set
+
+    def _session_get(self, key, default=None):
+        if key in self.strategy.session:
+            return self.strategy.session[key]
+
+        return default
+
+    def _session_set(self, key, value):
+        self.strategy.session[key] = value
+
+    @test.override_settings(SOCIAL_AUTH_LOGIN_REDIRECT_URL='dashboard_url')
+    def test_unsafe_redirect_url(self):
+        """
+        Test that if unsafe url is set as the redirect url then
+        redirect url is updated to SOCIAL_AUTH_LOGIN_REDIRECT_URL
+        """
+        self.strategy.session = {
+            REDIRECT_FIELD_NAME: 'https://evil.com',
+        }
+
+        pipeline.ensure_redirect_url_is_safe(self.strategy)
+        assert self.strategy.session[REDIRECT_FIELD_NAME] == 'dashboard_url'
+
+    @test.override_settings(LOGIN_REDIRECT_WHITELIST=['localhost:1991'])
+    def test_whitelisted_redirect_urls(self):
+        """
+        Test that redirect url remains same if its whitelisted
+        """
+        safe_redirect_url = 'https://localhost:1991/course'
+        self.strategy.session = {REDIRECT_FIELD_NAME: safe_redirect_url}
+
+        pipeline.ensure_redirect_url_is_safe(self.strategy)
+        assert self.strategy.session[REDIRECT_FIELD_NAME] == safe_redirect_url