fix: ensure redirect url is whitelisted
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user