From 72300c620af7b2ddbcc2097b58088f355364d65b Mon Sep 17 00:00:00 2001 From: Douglas Hall Date: Thu, 25 Oct 2018 13:13:25 -0400 Subject: [PATCH] Enable OAuth2 clients to logout with a redirect back to the client site. --- .../djangoapps/user_authn/tests/test_utils.py | 18 ++++++++++++++++++ openedx/core/djangoapps/user_authn/utils.py | 13 +++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/user_authn/tests/test_utils.py b/openedx/core/djangoapps/user_authn/tests/test_utils.py index 1cdf37ec52..0b0555b9a4 100644 --- a/openedx/core/djangoapps/user_authn/tests/test_utils.py +++ b/openedx/core/djangoapps/user_authn/tests/test_utils.py @@ -4,7 +4,9 @@ import ddt from django.test import TestCase from django.test.client import RequestFactory from django.test.utils import override_settings +from six.moves.urllib.parse import urlencode # pylint: disable=import-error +from openedx.core.djangoapps.oauth_dispatch.tests.factories import ApplicationFactory from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect @@ -31,3 +33,19 @@ class TestRedirectUtils(TestCase): req = self.request.get('/login', HTTP_HOST=host) actual_is_safe = is_safe_login_or_logout_redirect(req, url) self.assertEqual(actual_is_safe, expected_is_safe) + + @ddt.data( + ('https://test.com/test', 'https://test.com/test', 'edx.org', True), + ('https://test.com/test', 'https://fake.com', 'edx.org', False), + ) + @ddt.unpack + def test_safe_redirect_oauth2(self, client_redirect_uri, redirect_url, host, expected_is_safe): + """ Test safe redirect_url parameter when logging out OAuth2 client. """ + application = ApplicationFactory(redirect_uris=client_redirect_uri) + params = { + 'client_id': application.client_id, + 'redirect_url': redirect_url, + } + req = self.request.get('/logout?{}'.format(urlencode(params)), HTTP_HOST=host) + actual_is_safe = is_safe_login_or_logout_redirect(req, redirect_url) + self.assertEqual(actual_is_safe, expected_is_safe) diff --git a/openedx/core/djangoapps/user_authn/utils.py b/openedx/core/djangoapps/user_authn/utils.py index 14d0a806b2..52571d070f 100644 --- a/openedx/core/djangoapps/user_authn/utils.py +++ b/openedx/core/djangoapps/user_authn/utils.py @@ -3,16 +3,25 @@ Utility functions used during user authentication. """ from django.conf import settings from django.utils import http +from oauth2_provider.models import Application +from six.moves.urllib.parse import urlparse # pylint: disable=import-error def is_safe_login_or_logout_redirect(request, redirect_to): """ Determine if the given redirect URL/path is safe for redirection. """ - request_host = request.get_host() # e.g. 'courses.edx.org' - login_redirect_whitelist = set(getattr(settings, 'LOGIN_REDIRECT_WHITELIST', [])) + + request_host = request.get_host() # e.g. 'courses.edx.org' login_redirect_whitelist.add(request_host) + # Allow OAuth2 clients to redirect back to their site after logout. + dot_client_id = request.GET.get('client_id') + if dot_client_id: + application = Application.objects.get(client_id=dot_client_id) + if redirect_to in application.redirect_uris: + login_redirect_whitelist.add(urlparse(redirect_to).netloc) + is_safe_url = http.is_safe_url(redirect_to, allowed_hosts=login_redirect_whitelist, require_https=True) return is_safe_url