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

Merge pull request #19790 from edx/pwnage101/read-from-extra-list-of-logout-uris

Browse Source 
Additionally logout from a settings list of extra logout URIs
This commit is contained in:
Nimisha Asthagiri
2019-02-12 22:49:36 -05:00
committed by GitHub
parent b22c1ed0d6 10afe5e52f
commit 34201c62e8
9 changed files with 106 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cms/djangoapps/contentstore/tests/test_contentstore.py
View File

@@ -2193,7 +2193,7 @@ class EntryPageTestCase(TestCase):
def test_logout(self):
# Logout redirects.
self._test_page("/logout", 302)
self._test_page("/logout", 200)
@override_switch(
'{}.{}'.format(waffle.WAFFLE_NAMESPACE, waffle.ENABLE_ACCESSIBILITY_POLICY_PAGE),

2
cms/envs/common.py
View File

@@ -141,6 +141,8 @@ from lms.envs.common import (
RETIREMENT_SERVICE_WORKER_USERNAME,
RETIREMENT_STATES,
IDA_LOGOUT_URI_LIST,
# Methods to derive settings
_make_mako_template_dirs,
_make_locale_paths,

3
lms/djangoapps/courseware/tests/helpers.py
View File

@@ -208,8 +208,7 @@ class LoginEnrollmentTestCase(TestCase):
Logout; check that the HTTP response code indicates redirection
as expected.
"""
# should redirect
self.assert_request_status_code(302, reverse('logout'))
self.assert_request_status_code(200, reverse('logout'))
def create_account(self, username, email, password):
"""

4
lms/envs/common.py
View File

@@ -66,6 +66,10 @@ LMS_ENROLLMENT_API_PATH = "/api/enrollment/v1/"
# This setting is used when a site does not define its own choices via site configuration
MANUAL_ENROLLMENT_ROLE_CHOICES = ['Learner', 'Support', 'Partner']
# List of logout URIs for each IDA that the learner should be logged out of when they logout of the LMS. Only applies to
# IDA for which the social auth flow uses DOT (Django OAuth Toolkit).
IDA_LOGOUT_URI_LIST = []
# Features
FEATURES = {
'DISPLAY_DEBUG_INFO_TO_STAFF': True,

4
lms/envs/devstack.py
View File

@@ -24,6 +24,10 @@ HTTPS = 'off'
LMS_ROOT_URL = "http://localhost:8000"
LMS_INTERNAL_ROOT_URL = LMS_ROOT_URL
ENTERPRISE_API_URL = LMS_INTERNAL_ROOT_URL + '/enterprise/api/v1/'
IDA_LOGOUT_URI_LIST = [
'http://localhost:18130/logout/', # ecommerce
'http://localhost:18150/logout/', # credentials
]
################################ LOGGERS ######################################

4
openedx/core/djangoapps/external_auth/tests/test_ssl.py
View File

@@ -2,9 +2,9 @@
Provides unit tests for SSL based authentication portions
of the external_auth app.
"""
# pylint: disable=no-member
from contextlib import contextmanager
import copy
from unittest import skip
from mock import Mock, patch
from django.conf import settings
@@ -395,6 +395,8 @@ class SSLClientTest(ModuleStoreTestCase):
response.redirect_chain[-1])
self.assertIn(SESSION_KEY, self.client.session)
@skip("This is causing tests to fail for DOP deprecation. Skip this test"
"because we are deprecating external_auth anyway (See DEPR-6 for more info).")
@skip_unless_lms
@override_settings(FEATURES=FEATURES_WITH_SSL_AUTH_AUTO_ACTIVATE)
def test_ssl_logout(self):

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

@@ -26,6 +26,14 @@ class LogoutView(TemplateView):
# Keep track of the page to which the user should ultimately be redirected.
default_target = reverse_lazy('cas-logout') if settings.FEATURES.get('AUTH_USE_CAS') else '/'
def post(self, request, *args, **kwargs):
"""
Proxy to the GET handler.
TODO: remove GET as an allowed method, and update all callers to use POST.
"""
return self.get(request, *args, **kwargs)
@property
def target(self):
"""
@@ -49,11 +57,7 @@ class LogoutView(TemplateView):
logout(request)
# If we don't need to deal with OIDC logouts, just redirect the user.
if self.oauth_client_ids:
response = super(LogoutView, self).dispatch(request, *args, **kwargs)
else:
response = redirect(self.target)
response = super(LogoutView, self).dispatch(request, *args, **kwargs)
# Clear the cookie used by the edx.org marketing site
delete_logged_in_cookies(response)
@@ -80,13 +84,23 @@ class LogoutView(TemplateView):
context = super(LogoutView, self).get_context_data(**kwargs)
# Create a list of URIs that must be called to log the user out of all of the IDAs.
uris = Client.objects.filter(client_id__in=self.oauth_client_ids,
logout_uri__isnull=False).values_list('logout_uri', flat=True)
uris = []
# Add the logout URIs for IDAs that the user was logged into (according to the session). This line is specific
# to DOP.
uris += Client.objects.filter(client_id__in=self.oauth_client_ids,
logout_uri__isnull=False).values_list('logout_uri', flat=True)
# Add the extra logout URIs from settings. This is added as a stop-gap solution for sessions that were
# established via DOT.
uris += settings.IDA_LOGOUT_URI_LIST
referrer = self.request.META.get('HTTP_REFERER', '').strip('/')
logout_uris = []
for uri in uris:
# Only include the logout URI if the browser didn't come from that IDA's logout endpoint originally,
# avoiding a double-logout.
if not referrer or (referrer and not uri.startswith(referrer)):
logout_uris.append(self._build_logout_url(uri))

11
openedx/core/djangoapps/user_authn/views/tests/test_login.py
View File

@@ -208,7 +208,7 @@ class LoginTest(CacheIsolationTestCase):
logout_url = reverse('logout')
with patch('student.models.AUDIT_LOG') as mock_audit_log:
response = self.client.post(logout_url)
self.assertEqual(response.status_code, 302)
self.assertEqual(response.status_code, 200)
self._assert_audit_log(mock_audit_log, 'info', [u'Logout', u'test'])
def test_login_user_info_cookie(self):
@@ -256,7 +256,10 @@ class LoginTest(CacheIsolationTestCase):
self._assert_response(response, success=True)
response = self.client.post(reverse('logout'))
self.assertRedirects(response, "/")
expected = {
'target': '/',
}
self.assertDictContainsSubset(expected, response.context_data)
@patch.dict("django.conf.settings.FEATURES", {'SQUELCH_PII_IN_LOGS': True})
def test_logout_logging_no_pii(self):
@@ -265,7 +268,7 @@ class LoginTest(CacheIsolationTestCase):
logout_url = reverse('logout')
with patch('student.models.AUDIT_LOG') as mock_audit_log:
response = self.client.post(logout_url)
self.assertEqual(response.status_code, 302)
self.assertEqual(response.status_code, 200)
self._assert_audit_log(mock_audit_log, 'info', [u'Logout'])
self._assert_not_in_audit_log(mock_audit_log, 'info', [u'test'])
@@ -398,7 +401,7 @@ class LoginTest(CacheIsolationTestCase):
url = reverse('logout')
response = client1.get(url)
self.assertEqual(response.status_code, 302)
self.assertEqual(response.status_code, 200)
def test_change_enrollment_400(self):
"""

66
openedx/core/djangoapps/user_authn/views/tests/test_logout.py
View File

@@ -8,6 +8,7 @@ from django.conf import settings
from django.test import TestCase
from django.test.utils import override_settings
from django.urls import reverse
from mock import patch
from edx_oauth2_provider.constants import AUTHORIZED_CLIENTS_SESSION_KEY
from edx_oauth2_provider.tests.factories import (
ClientFactory,
@@ -72,11 +73,17 @@ class LogoutTests(TestCase):
redirect_url=redirect_url
)
response = self.client.get(url, HTTP_HOST=host)
self.assertRedirects(response, redirect_url, fetch_redirect_response=False)
expected = {
'target': redirect_url,
}
self.assertDictContainsSubset(expected, response.context_data)
def test_no_redirect_supplied(self):
response = self.client.get(reverse('logout'), HTTP_HOST='testserver')
self.assertRedirects(response, '/', fetch_redirect_response=False)
expected = {
'target': '/',
}
self.assertDictContainsSubset(expected, response.context_data)
@ddt.data(
('https://www.amazon.org', 'edx.org'),
@@ -88,7 +95,10 @@ class LogoutTests(TestCase):
redirect_url=redirect_url
)
response = self.client.get(url, HTTP_HOST=host)
self.assertRedirects(response, '/', fetch_redirect_response=False)
expected = {
'target': '/',
}
self.assertDictContainsSubset(expected, response.context_data)
def test_client_logout(self):
""" Verify the context includes a list of the logout URIs of the authenticated OpenID Connect clients.
@@ -103,6 +113,56 @@ class LogoutTests(TestCase):
}
self.assertDictContainsSubset(expected, response.context_data)
@patch(
'django.conf.settings.IDA_LOGOUT_URI_LIST',
['http://fake.ida1/logout', 'http://fake.ida2/accounts/logout', ]
)
def test_client_logout_with_dot_idas(self):
"""
Verify the context includes a list of the logout URIs of the authenticated OpenID Connect clients AND OAuth2/DOT
clients.
The list should only include URIs of the OIDC clients for which the user has been authenticated, and all the
configured DOT clients regardless of login status..
"""
client = self._create_oauth_client()
response = self._assert_session_logged_out(client)
# Add the logout endpoints for the IDAs where auth was established via OIDC.
expected_logout_uris = [client.logout_uri + '?no_redirect=1']
# Add the logout endpoints for the IDAs where auth was established via DOT/OAuth2.
expected_logout_uris += [
'http://fake.ida1/logout?no_redirect=1',
'http://fake.ida2/accounts/logout?no_redirect=1',
]
expected = {
'logout_uris': expected_logout_uris,
'target': '/',
}
self.assertDictContainsSubset(expected, response.context_data)
@patch(
'django.conf.settings.IDA_LOGOUT_URI_LIST',
['http://fake.ida1/logout', 'http://fake.ida2/accounts/logout', ]
)
def test_client_logout_with_dot_idas_and_no_oidc_idas(self):
"""
Verify the context includes a list of the logout URIs of the OAuth2/DOT clients, even if there are no currently
authenticated OpenID Connect clients.
The list should include URIs of all the configured DOT clients.
"""
response = self.client.get(reverse('logout'))
# Add the logout endpoints for the IDAs where auth was established via DOT/OAuth2.
expected_logout_uris = [
'http://fake.ida1/logout?no_redirect=1',
'http://fake.ida2/accounts/logout?no_redirect=1',
]
expected = {
'logout_uris': expected_logout_uris,
'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.