Skip to content

Commit

Permalink
fix: patch open redirect
Browse files Browse the repository at this point in the history
  • Loading branch information
Rebecca Graber committed Apr 6, 2022
1 parent 99bcdb6 commit fbbcfe7
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 2 deletions.
58 changes: 58 additions & 0 deletions common/djangoapps/third_party_auth/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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')
12 changes: 10 additions & 2 deletions common/djangoapps/third_party_auth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit fbbcfe7

Please sign in to comment.