diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 3a73f70d9f..cc931df4ec 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/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= + + 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) diff --git a/common/djangoapps/third_party_auth/settings.py b/common/djangoapps/third_party_auth/settings.py index 2d714704c4..73332b7acc 100644 --- a/common/djangoapps/third_party_auth/settings.py +++ b/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 diff --git a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py index e65932afb0..7b26cb041a 100644 --- a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py +++ b/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