From 0c164ad4c24d8ee2a5d5f0a35cd057f21581866d Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Fri, 10 Sep 2021 18:45:58 +0000 Subject: [PATCH] fix: Fix Studio logout by pointing to correct logout view (#28714) This changes the "Sign out" link on Studio to point to Studio's own logout view, which clears the session and then redirects to LMS's logout page. The LMS logout page then skips loading the Studio logout because it is seen in the Referer header. This change also brings Studio better into line with how other IDAs perform their logouts. Background: After the rollout of Studio OAuth, logouts initiated on Studio failed to actually log out Studio (but all other IDAs were logged out). This was because the LMS logout view loads the logout pages of other IDAs but skips any that is a *prefix* match on the Referer header, and browsers now often send a truncated version of the Referer for privacy. Therefore, Studio was always skipped when coming from Studio. The fix is to make sure that Studio has already performed its logout by the time the LMS logout page is loaded. One wrinkle here is that the LMS logout view is activated by `/logout`, but the correct logout view (provided by auth_backends) is activated by `/logout/` -- with a trailing slash. This is fragile and unfortunate, but can be cleaned up when we later remove other leftovers of Studio's previous ability to handle logistration. ref: ARCHBOM-1897 --- cms/envs/common.py | 5 +++-- cms/envs/devstack.py | 1 - cms/urls.py | 2 ++ docs/guides/studio_oauth.rst | 8 ++++++++ openedx/core/djangoapps/user_authn/urls_common.py | 3 +++ 5 files changed, 16 insertions(+), 3 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 5af3d7ab9b..e3e0dcfb6c 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -652,8 +652,9 @@ LOGIN_REDIRECT_URL = EDX_ROOT_URL + '/home/' LOGIN_URL = reverse_lazy('login_redirect_to_lms') FRONTEND_LOGIN_URL = lambda settings: settings.LMS_ROOT_URL + '/login' derived('FRONTEND_LOGIN_URL') -FRONTEND_LOGOUT_URL = lambda settings: settings.LMS_ROOT_URL + '/logout' -derived('FRONTEND_LOGOUT_URL') +# Warning: Must have trailing slash to activate correct logout view +# (auth_backends, not LMS user_authn) +FRONTEND_LOGOUT_URL = '/logout/' FRONTEND_REGISTER_URL = lambda settings: settings.LMS_ROOT_URL + '/register' derived('FRONTEND_REGISTER_URL') diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 70b966a8b9..1bfc970beb 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -51,7 +51,6 @@ FEATURES['PREVIEW_LMS_BASE'] = "preview." + LMS_BASE # TODO: Remove after Studio OAuth transition is complete. See docs/guides/studio_oauth.rst LOGIN_URL = '/login/' FRONTEND_LOGIN_URL = LMS_ROOT_URL + '/login' -FRONTEND_LOGOUT_URL = LMS_ROOT_URL + '/logout' FRONTEND_REGISTER_URL = LMS_ROOT_URL + '/register' ########################### PIPELINE ################################# diff --git a/cms/urls.py b/cms/urls.py index fa0fe871b0..d82cece189 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -44,6 +44,8 @@ COURSELIKE_KEY_PATTERN = r'(?P({}|{}))'.format( # Pattern to match a library key only LIBRARY_KEY_PATTERN = r'(?Plibrary-v1:[^/+]+\+[^/+]+)' +# oauth2_urlpatterns needs to be first to override any other login and +# logout related views. urlpatterns = oauth2_urlpatterns + [ url(r'', include('openedx.core.djangoapps.user_authn.urls_common')), url(r'', include('common.djangoapps.student.urls')), diff --git a/docs/guides/studio_oauth.rst b/docs/guides/studio_oauth.rst index f0d6b2a892..8f0d381e85 100644 --- a/docs/guides/studio_oauth.rst +++ b/docs/guides/studio_oauth.rst @@ -65,3 +65,11 @@ Config and code changes to be performed after all environments are using OAuth f - Remove ``LOGIN_URL`` overrides from all environments (devstack and others) - Remove remaining ``ARCH-1253`` detritus (login redirect) - Remove this doc! + +Declining the migration +----------------------- + +Untested instructions for continuing to keep the shared sessions: + +- Override ``FRONTEND_LOGOUT_URL`` for Studio to be ``/logout`` +- Override ``LOGIN_URL`` for Studio to be ``/login`` diff --git a/openedx/core/djangoapps/user_authn/urls_common.py b/openedx/core/djangoapps/user_authn/urls_common.py index 0268332907..0fce4a5e67 100644 --- a/openedx/core/djangoapps/user_authn/urls_common.py +++ b/openedx/core/djangoapps/user_authn/urls_common.py @@ -65,6 +65,9 @@ urlpatterns = [ # Login Refresh of JWT Cookies url(r'^login_refresh$', login.login_refresh, name="login_refresh"), + # WARNING: This is similar to auth_backends ^logout/$ (which has a + # trailing slash); LMS uses this view, but Studio links to the + # auth_backends logout view. url(r'^logout$', logout.LogoutView.as_view(), name='logout'), # Moved from user_api/legacy_urls.py