From fbbcfe71832e700f16aad3636b0ccb35585d1c95 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Wed, 6 Apr 2022 15:42:46 -0400 Subject: [PATCH] fix: patch open redirect --- .../third_party_auth/tests/test_views.py | 58 +++++++++++++++++++ common/djangoapps/third_party_auth/views.py | 12 +++- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/third_party_auth/tests/test_views.py b/common/djangoapps/third_party_auth/tests/test_views.py index 3508a9d941b6..10c64579ebbf 100644 --- a/common/djangoapps/third_party_auth/tests/test_views.py +++ b/common/djangoapps/third_party_auth/tests/test_views.py @@ -4,17 +4,23 @@ import unittest +from unittest.mock import patch import ddt import pytest from django.conf import settings +from django.test import TestCase, override_settings +from django.test.client import RequestFactory from django.urls import reverse from lxml import etree from onelogin.saml2.errors import OneLogin_Saml2_Error +from common.djangoapps.student.models import Registration +from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.third_party_auth import pipeline # Define some XML namespaces: from common.djangoapps.third_party_auth.tasks import SAML_XML_NS +from common.djangoapps.third_party_auth.views import inactive_user_view from .testutil import AUTH_FEATURE_ENABLED, AUTH_FEATURES_KEY, SAMLTestCase @@ -196,3 +202,55 @@ def get_idp_redirect_url(provider_slug, next_destination=None): idp_redirect_url=reverse('idp_redirect', kwargs={'provider_slug': provider_slug}), next_destination=next_destination, ) + + +@unittest.skipUnless(AUTH_FEATURE_ENABLED, AUTH_FEATURES_KEY + ' not enabled') +class InactiveUserViewTests(TestCase): + """Test inactive user view """ + @patch('common.djangoapps.third_party_auth.views.redirect') + @override_settings(LOGIN_REDIRECT_WHITELIST=['courses.edx.org']) + def test_inactive_user_view_allows_valid_redirect(self, mock_redirect): + inactive_user = UserFactory(is_active=False) + Registration().register(inactive_user) + request = RequestFactory().get(settings.SOCIAL_AUTH_INACTIVE_USER_URL, {'next': 'https://courses.edx.org'}) + request.user = inactive_user + with patch('common.djangoapps.edxmako.request_context.get_current_request', return_value=request): + with patch('common.djangoapps.third_party_auth.pipeline.running', return_value=False): + inactive_user_view(request) + mock_redirect.assert_called_with('https://courses.edx.org') + + @patch('common.djangoapps.third_party_auth.views.redirect') + def test_inactive_user_view_prevents_invalid_redirect(self, mock_redirect): + inactive_user = UserFactory(is_active=False) + Registration().register(inactive_user) + request = RequestFactory().get(settings.SOCIAL_AUTH_INACTIVE_USER_URL, {'next': 'https://evil.com'}) + request.user = inactive_user + with patch('common.djangoapps.edxmako.request_context.get_current_request', return_value=request): + with patch('common.djangoapps.third_party_auth.pipeline.running', return_value=False): + inactive_user_view(request) + mock_redirect.assert_called_with('dashboard') + + @patch('common.djangoapps.third_party_auth.views.redirect') + def test_inactive_user_view_redirects_back_to_host(self, mock_redirect): + inactive_user = UserFactory(is_active=False) + Registration().register(inactive_user) + request = RequestFactory().get(settings.SOCIAL_AUTH_INACTIVE_USER_URL, {'next': 'https://myedxhost.com'}, + HTTP_HOST='myedxhost.com') + request.user = inactive_user + with patch('common.djangoapps.edxmako.request_context.get_current_request', return_value=request): + with patch('common.djangoapps.third_party_auth.pipeline.running', return_value=False): + inactive_user_view(request) + mock_redirect.assert_called_with('https://myedxhost.com') + + @patch('common.djangoapps.third_party_auth.views.redirect') + @override_settings(LOGIN_REDIRECT_WHITELIST=['courses.edx.org']) + def test_inactive_user_view_does_not_redirect_https_to_http(self, mock_redirect): + inactive_user = UserFactory(is_active=False) + Registration().register(inactive_user) + request = RequestFactory().get(settings.SOCIAL_AUTH_INACTIVE_USER_URL, {'next': 'http://courses.edx.org'}, + secure=True) + request.user = inactive_user + with patch('common.djangoapps.edxmako.request_context.get_current_request', return_value=request): + with patch('common.djangoapps.third_party_auth.pipeline.running', return_value=False): + inactive_user_view(request) + mock_redirect.assert_called_with('dashboard') diff --git a/common/djangoapps/third_party_auth/views.py b/common/djangoapps/third_party_auth/views.py index 3f35a48a326b..c6a406c5301c 100644 --- a/common/djangoapps/third_party_auth/views.py +++ b/common/djangoapps/third_party_auth/views.py @@ -14,7 +14,7 @@ from social_django.views import complete from common.djangoapps import third_party_auth -from common.djangoapps.student.helpers import get_next_url_for_login_page +from common.djangoapps.student.helpers import get_next_url_for_login_page, is_safe_login_or_logout_redirect from common.djangoapps.student.models import UserProfile from common.djangoapps.student.views import compose_and_send_activation_email from common.djangoapps.third_party_auth import pipeline, provider @@ -54,7 +54,15 @@ def inactive_user_view(request): if not activated: compose_and_send_activation_email(user, profile) - return redirect(request.GET.get('next', 'dashboard')) + request_params = request.GET + redirect_to = request_params.get('next') + + if redirect_to and is_safe_login_or_logout_redirect(redirect_to=redirect_to, request_host=request.get_host(), + dot_client_id=request_params.get('client_id'), + require_https=request.is_secure()): + return redirect(redirect_to) + + return redirect('dashboard') def saml_metadata_view(request):