Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #18989 from edx/robrap/ARCH-241-logout-redirect

Browse Source 
ARCH-241: Add ability to redirect to subdomain for logout.
This commit is contained in:
Robert Raposa
2018-10-03 15:06:11 -04:00
committed by GitHub
parent d447c6c9b5 027c53e61e
commit eca340d2e1
5 changed files with 146 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
common/djangoapps/student/helpers.py
View File

@@ -326,7 +326,7 @@ def _get_redirect_to(request):
# get information about a user on edx.org. In any such case drop the parameter.
if redirect_to:
mime_type, _ = mimetypes.guess_type(redirect_to, strict=False)
if not _is_safe_redirect(request, redirect_to):
if not is_safe_redirect(request, redirect_to):
log.warning(
u'Unsafe redirect parameter detected after login page: %(redirect_to)r',
{"redirect_to": redirect_to}
@@ -369,7 +369,7 @@ def _get_redirect_to(request):
return redirect_to
def _is_safe_redirect(request, redirect_to):
def is_safe_redirect(request, redirect_to):
"""
Determine if the given redirect URL/path is safe for redirection.
"""

35
common/djangoapps/student/tests/test_helpers.py
View File

@@ -9,11 +9,10 @@ from django.urls import reverse
from django.test import TestCase
from django.test.client import RequestFactory
from django.test.utils import override_settings
from django.utils import http
from mock import patch
from testfixtures import LogCapture
from student.helpers import get_next_url_for_login_page
from student.helpers import get_next_url_for_login_page, is_safe_redirect
from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration_context
LOGGER_NAME = "student.helpers"
@@ -53,7 +52,7 @@ class TestLoginHelper(TestCase):
"Redirect to url path with specified filed type 'image/png' not allowed: u'" + static_url + "dummy.png" + "'"),
)
@ddt.unpack
def test_unsafe_next(self, log_level, log_name, unsafe_url, http_accept, user_agent, expected_log):
def test_next_failures(self, log_level, log_name, unsafe_url, http_accept, user_agent, expected_log):
""" Test unsafe next parameter """
with LogCapture(LOGGER_NAME, level=log_level) as logger:
req = self.request.get(reverse("login") + "?next={url}".format(url=unsafe_url))
@@ -65,19 +64,35 @@ class TestLoginHelper(TestCase):
)
@ddt.data(
('/dashboard', 'testserver', '/dashboard'),
('https://edx.org/courses', 'edx.org', 'https://edx.org/courses'),
('https://test.edx.org/courses', 'edx.org', 'https://test.edx.org/courses'),
('https://test2.edx.org/courses', 'edx.org', 'https://test2.edx.org/courses'),
('/dashboard', 'testserver'),
('https://edx.org/courses', 'edx.org'),
('https://test.edx.org/courses', 'edx.org'),
('https://test2.edx.org/courses', 'edx.org'),
)
@ddt.unpack
@override_settings(LOGIN_REDIRECT_WHITELIST=['test.edx.org', 'test2.edx.org'])
def test_safe_next(self, url, host, expected_url):
def test_safe_next(self, next_url, host):
""" Test safe next parameter """
req = self.request.get(reverse("login") + "?next={url}".format(url=url), HTTP_HOST=host)
req = self.request.get(reverse("login") + "?next={url}".format(url=next_url), HTTP_HOST=host)
req.META["HTTP_ACCEPT"] = "text/html" # pylint: disable=no-member
next_page = get_next_url_for_login_page(req)
self.assertEqual(next_page, expected_url)
self.assertEqual(next_page, next_url)
@ddt.data(
('/dashboard', 'testserver', True),
('https://edx.org/courses', 'edx.org', True),
('https://test.edx.org/courses', 'edx.org', True),
('https://www.amazon.org', 'edx.org', False),
('http://edx.org/courses', 'edx.org', False),
('http:///edx.org/courses', 'edx.org', False), # Django's is_safe_url protects against "///"
)
@ddt.unpack
@override_settings(LOGIN_REDIRECT_WHITELIST=['test.edx.org'])
def test_safe_redirect(self, url, host, expected_is_safe):
""" Test safe next parameter """
req = self.request.get(reverse("login"), HTTP_HOST=host)
actual_is_safe = is_safe_redirect(req, url)
self.assertEqual(actual_is_safe, expected_is_safe)
@patch('student.helpers.third_party_auth.pipeline.get')
@ddt.data(

88
common/djangoapps/student/tests/test_views.py
View File

@@ -19,9 +19,6 @@ from opaque_keys import InvalidKeyError
from bulk_email.models import BulkEmailFlag
from course_modes.models import CourseMode
from edx_oauth2_provider.constants import AUTHORIZED_CLIENTS_SESSION_KEY
from edx_oauth2_provider.tests.factories import (ClientFactory,
TrustedClientFactory)
from entitlements.tests.factories import CourseEntitlementFactory
from milestones.tests.utils import MilestonesTestCaseMixin
from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory
@@ -157,91 +154,6 @@ class TestStudentDashboardUnenrollments(SharedModuleStoreTestCase):
self.assertEqual(response.status_code, 406)
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class LogoutTests(TestCase):
""" Tests for the logout functionality. """
def setUp(self):
""" Create a course and user, then log in. """
super(LogoutTests, self).setUp()
self.user = UserFactory()
self.client.login(username=self.user.username, password=PASSWORD)
def create_oauth_client(self):
""" Creates a trusted OAuth client. """
client = ClientFactory(logout_uri='https://www.example.com/logout/')
TrustedClientFactory(client=client)
return client
def assert_session_logged_out(self, oauth_client, **logout_headers):
""" Authenticates a user via OAuth 2.0, logs out, and verifies the session is logged out. """
self.authenticate_with_oauth(oauth_client)
# Logging out should remove the session variables, and send a list of logout URLs to the template.
# The template will handle loading those URLs and redirecting the user. That functionality is not tested here.
response = self.client.get(reverse('logout'), **logout_headers)
self.assertEqual(response.status_code, 200)
self.assertNotIn(AUTHORIZED_CLIENTS_SESSION_KEY, self.client.session)
return response
def authenticate_with_oauth(self, oauth_client):
""" Perform an OAuth authentication using the current web client.
This should add an AUTHORIZED_CLIENTS_SESSION_KEY entry to the current session.
"""
data = {
'client_id': oauth_client.client_id,
'client_secret': oauth_client.client_secret,
'response_type': 'code'
}
# Authenticate with OAuth to set the appropriate session values
self.client.post(reverse('oauth2:capture'), data, follow=True)
self.assertListEqual(self.client.session[AUTHORIZED_CLIENTS_SESSION_KEY], [oauth_client.client_id])
def assert_logout_redirects_to_root(self):
""" Verify logging out redirects the user to the homepage. """
response = self.client.get(reverse('logout'))
self.assertRedirects(response, '/', fetch_redirect_response=False)
def assert_logout_redirects_with_target(self):
""" Verify logging out with a redirect_url query param redirects the user to the target. """
url = '{}?{}'.format(reverse('logout'), 'redirect_url=/courses')
response = self.client.get(url)
self.assertRedirects(response, '/courses', fetch_redirect_response=False)
def test_without_session_value(self):
""" Verify logout works even if the session does not contain an entry with
the authenticated OpenID Connect clients."""
self.assert_logout_redirects_to_root()
self.assert_logout_redirects_with_target()
def test_client_logout(self):
""" Verify the context includes a list of the logout URIs of the authenticated OpenID Connect clients.
The list should only include URIs of the clients for which the user has been authenticated.
"""
client = self.create_oauth_client()
response = self.assert_session_logged_out(client)
expected = {
'logout_uris': [client.logout_uri + '?no_redirect=1'], # pylint: disable=no-member
'target': '/',
}
self.assertDictContainsSubset(expected, response.context_data) # pylint: disable=no-member
def test_filter_referring_service(self):
""" Verify that, if the user is directed to the logout page from a service, that service's logout URL
is not included in the context sent to the template.
"""
client = self.create_oauth_client()
response = self.assert_session_logged_out(client, HTTP_REFERER=client.logout_uri) # pylint: disable=no-member
expected = {
'logout_uris': [],
'target': '/',
}
self.assertDictContainsSubset(expected, response.context_data) # pylint: disable=no-member
@ddt.ddt
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, CompletionWaffleTestMixin):

9
openedx/core/djangoapps/user_authn/views/logout.py
View File

@@ -6,10 +6,11 @@ from django.conf import settings
from django.contrib.auth import logout
from django.urls import reverse_lazy
from django.shortcuts import redirect
from django.utils.http import is_safe_url, urlencode
from django.utils.http import urlencode
from django.views.generic import TemplateView
from provider.oauth2.models import Client
from openedx.core.djangoapps.user_authn.cookies import delete_logged_in_cookies
from student.helpers import is_safe_redirect
class LogoutView(TemplateView):
@@ -34,11 +35,7 @@ class LogoutView(TemplateView):
"""
target_url = self.request.GET.get('redirect_url')
if target_url and is_safe_url(
target_url,
allowed_hosts={self.request.META.get('HTTP_HOST')},
require_https=True,
):
if target_url and is_safe_redirect(self.request, target_url):
return target_url
else:
return self.default_target

116
openedx/core/djangoapps/user_authn/views/tests/test_logout.py Normal file
View File

@@ -0,0 +1,116 @@
"""
Tests for logout
"""
import unittest
import ddt
from django.conf import settings
from django.test import TestCase
from django.test.utils import override_settings
from django.urls import reverse
from edx_oauth2_provider.constants import AUTHORIZED_CLIENTS_SESSION_KEY
from edx_oauth2_provider.tests.factories import (
ClientFactory,
TrustedClientFactory
)
from student.tests.factories import UserFactory
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
@ddt.ddt
class LogoutTests(TestCase):
""" Tests for the logout functionality. """
def setUp(self):
""" Create a course and user, then log in. """
super(LogoutTests, self).setUp()
self.user = UserFactory()
self.client.login(username=self.user.username, password='test')
def _create_oauth_client(self):
""" Creates a trusted OAuth client. """
client = ClientFactory(logout_uri='https://www.example.com/logout/')
TrustedClientFactory(client=client)
return client
def _assert_session_logged_out(self, oauth_client, **logout_headers):
""" Authenticates a user via OAuth 2.0, logs out, and verifies the session is logged out. """
self._authenticate_with_oauth(oauth_client)
# Logging out should remove the session variables, and send a list of logout URLs to the template.
# The template will handle loading those URLs and redirecting the user. That functionality is not tested here.
response = self.client.get(reverse('logout'), **logout_headers)
self.assertEqual(response.status_code, 200)
self.assertNotIn(AUTHORIZED_CLIENTS_SESSION_KEY, self.client.session)
return response
def _authenticate_with_oauth(self, oauth_client):
""" Perform an OAuth authentication using the current web client.
This should add an AUTHORIZED_CLIENTS_SESSION_KEY entry to the current session.
"""
data = {
'client_id': oauth_client.client_id,
'client_secret': oauth_client.client_secret,
'response_type': 'code'
}
# Authenticate with OAuth to set the appropriate session values
self.client.post(reverse('oauth2:capture'), data, follow=True)
self.assertListEqual(self.client.session[AUTHORIZED_CLIENTS_SESSION_KEY], [oauth_client.client_id])
@ddt.data(
('/courses', 'testserver'),
('https://edx.org/courses', 'edx.org'),
('https://test.edx.org/courses', 'edx.org'),
)
@ddt.unpack
@override_settings(LOGIN_REDIRECT_WHITELIST=['test.edx.org'])
def test_logout_redirect_success(self, redirect_url, host):
url = '{logout_path}?redirect_url={redirect_url}'.format(
logout_path=reverse('logout'),
redirect_url=redirect_url
)
response = self.client.get(url, HTTP_HOST=host)
self.assertRedirects(response, redirect_url, fetch_redirect_response=False)
def test_no_redirect_supplied(self):
response = self.client.get(reverse('logout'), HTTP_HOST='testserver')
self.assertRedirects(response, '/', fetch_redirect_response=False)
@ddt.data(
('https://www.amazon.org', 'edx.org'),
)
@ddt.unpack
def test_logout_redirect_failure(self, redirect_url, host):
url = '{logout_path}?redirect_url={redirect_url}'.format(
logout_path=reverse('logout'),
redirect_url=redirect_url
)
response = self.client.get(url, HTTP_HOST=host)
self.assertRedirects(response, '/', fetch_redirect_response=False)
def test_client_logout(self):
""" Verify the context includes a list of the logout URIs of the authenticated OpenID Connect clients.
The list should only include URIs of the clients for which the user has been authenticated.
"""
client = self._create_oauth_client()
response = self._assert_session_logged_out(client)
expected = {
'logout_uris': [client.logout_uri + '?no_redirect=1'],
'target': '/',
}
self.assertDictContainsSubset(expected, response.context_data)
def test_filter_referring_service(self):
""" Verify that, if the user is directed to the logout page from a service, that service's logout URL
is not included in the context sent to the template.
"""
client = self._create_oauth_client()
response = self._assert_session_logged_out(client, HTTP_REFERER=client.logout_uri)
expected = {
'logout_uris': [],
'target': '/',
}
self.assertDictContainsSubset(expected, response.context_data)