diff --git a/cms/djangoapps/contentstore/middleware.py b/cms/djangoapps/contentstore/middleware.py new file mode 100644 index 000000000000..f736d64be9b4 --- /dev/null +++ b/cms/djangoapps/contentstore/middleware.py @@ -0,0 +1,64 @@ +""" +Middleware for Studio. +""" + +from urllib.parse import urlsplit + +from django.conf import settings +from django.http import HttpResponse +from django.shortcuts import resolve_url +from edx_django_utils.monitoring import set_custom_attribute + + +class LoginRedirectToUnauthorizedMiddleware: + """ + Answer 401 instead of redirecting to login when the client cannot use a login page. + + Studio views are mostly protected by ``login_required``, which redirects an + unauthenticated request to ``LOGIN_URL``. That is right for a page navigation + and wrong for a request made by a script: the script follows the redirect, + and with LMS SSO each such request starts an OAuth login of its own. A page + that sends many requests at once after its Studio session has expired starts + that many logins against one session. They overwrite each other's ``state``, + and all but a few fail in ``social_core`` with ``AuthStateForbidden`` or + ``AuthStateMissing``, which Studio serves as a 500. + + A 401 lets the caller start a single login instead. The test is the one + ``SafeSessionMiddleware`` uses when it rejects a session: only a client that + asks for HTML is redirected. Unlike there, a request with no ``Accept`` + header at all keeps the redirect, so only clients that say what they want + are affected. + """ + + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + response = self.get_response(request) + if self._is_unusable_login_redirect(request, response): + # .. custom_attribute_name: studio_login_redirect_replaced + # .. custom_attribute_description: Set to True when a redirect to login was + # replaced by a 401 because the client did not ask for HTML. + set_custom_attribute('studio_login_redirect_replaced', True) + return HttpResponse(status=401) + return response + + @staticmethod + def _is_unusable_login_redirect(request, response): + """ + Return whether the response sends an anonymous, non-HTML client to the login page. + """ + if response.status_code != 302 or not response.has_header('Location'): + return False + + user = getattr(request, 'user', None) + if user is not None and user.is_authenticated: + return False + + accept = request.headers.get('Accept') + if accept is None or 'text/html' in accept: + return False + + location = urlsplit(response['Location']) + login_url = urlsplit(resolve_url(settings.LOGIN_URL)) + return location.path == login_url.path and location.netloc in ('', login_url.netloc, request.get_host()) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 9ce85d525d26..8b80f83f2933 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -570,7 +570,7 @@ def test_creation(self): # Now log out and ensure we are forbidden from creating a library: self.client.logout() - self._assert_cannot_create_library(expected_code=302) # 302 redirect to login expected + self._assert_cannot_create_library(expected_code=401) # an AJAX request gets 401, not the login redirect # Now check that logged-in users without CourseCreator role cannot create libraries self._login_as_non_staff_user(logout_first=False) diff --git a/cms/djangoapps/contentstore/tests/test_middleware.py b/cms/djangoapps/contentstore/tests/test_middleware.py new file mode 100644 index 000000000000..a7479c2ab28b --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_middleware.py @@ -0,0 +1,89 @@ +""" +Tests for Studio's middleware. +""" + +import ddt +from django.contrib.auth.views import redirect_to_login +from django.http import HttpResponse, HttpResponseRedirect +from django.test import RequestFactory, TestCase, override_settings +from django.urls import reverse + +from cms.djangoapps.contentstore.middleware import LoginRedirectToUnauthorizedMiddleware +from common.djangoapps.student.tests.factories import UserFactory + +JSON_ACCEPT = 'application/json, text/plain, */*' +HTML_ACCEPT = 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8' +USAGE_KEY = 'block-v1:edX+DemoX+Demo_Course+type@vertical+block@abc123' + + +@ddt.ddt +class LoginRedirectToUnauthorizedMiddlewareTest(TestCase): + """ + Tests for LoginRedirectToUnauthorizedMiddleware. + """ + + + def setUp(self): + super().setUp() + self.outline_url = reverse('xblock_outline_handler', kwargs={'usage_key_string': USAGE_KEY}) + + @ddt.data(JSON_ACCEPT, 'application/json', '*/*') + def test_anonymous_script_request_gets_401(self, accept): + response = self.client.get(self.outline_url, HTTP_ACCEPT=accept) + + assert response.status_code == 401 + assert not response.has_header('Location') + + @ddt.data(HTML_ACCEPT, 'text/html') + def test_anonymous_page_request_is_redirected_to_login(self, accept): + response = self.client.get(self.outline_url, HTTP_ACCEPT=accept) + + assert response.status_code == 302 + assert response['Location'].startswith('/login/?next=') + + def test_anonymous_script_post_gets_401(self): + url = reverse('xblock_handler', kwargs={'usage_key_string': USAGE_KEY}) + + response = self.client.post(url, data='{}', content_type='application/json', HTTP_ACCEPT=JSON_ACCEPT) + + assert response.status_code == 401 + + @ddt.data('https://studio.example.com/login/', 'login') + def test_login_url_setting_forms_are_recognised(self, login_url): + with override_settings(LOGIN_URL=login_url): + response = self.client.get(self.outline_url, HTTP_ACCEPT=JSON_ACCEPT) + + assert response.status_code == 401 + + def test_anonymous_request_without_accept_header_is_redirected_to_login(self): + response = self.client.get(self.outline_url) + + assert response.status_code == 302 + assert response['Location'].startswith('/login/?next=') + + def test_authenticated_request_is_not_replaced(self): + request = RequestFactory().get('/some/path', HTTP_ACCEPT=JSON_ACCEPT) + request.user = UserFactory.create() + view_response = redirect_to_login('/some/path') + middleware = LoginRedirectToUnauthorizedMiddleware(lambda _request: view_response) + + assert middleware(request) is view_response + + @ddt.data( + HttpResponseRedirect('/home/'), + HttpResponseRedirect('https://example.com/login/'), + HttpResponse(status=403), + HttpResponse(status=200), + ) + def test_other_responses_pass_through(self, view_response): + request = RequestFactory().get('/some/path', HTTP_ACCEPT=JSON_ACCEPT) + middleware = LoginRedirectToUnauthorizedMiddleware(lambda _request: view_response) + + assert middleware(request) is view_response + + def test_login_redirect_on_request_host_is_replaced(self): + request = RequestFactory().get('/some/path', HTTP_ACCEPT=JSON_ACCEPT) + view_response = HttpResponseRedirect(f'http://{request.get_host()}/login/?next=/some/path') + middleware = LoginRedirectToUnauthorizedMiddleware(lambda _request: view_response) + + assert middleware(request).status_code == 401 diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index 8c33ad3f891f..dc184885b3f6 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -291,7 +291,7 @@ def test_reindex_course(self): self.client.logout() response = self.client.get(index_url, {}, HTTP_ACCEPT='application/json') - self.assertEqual(response.status_code, 302) # noqa: PT009 + self.assertEqual(response.status_code, 401) # noqa: PT009 def test_negative_conditions(self): """ diff --git a/cms/envs/common.py b/cms/envs/common.py index 87ee13f3e0a6..0c6661ae752b 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -406,6 +406,9 @@ def make_lms_template_path(settings): # Instead of AuthenticationMiddleware, we use a cache-backed version 'openedx.core.djangoapps.cache_toolbox.middleware.CacheBackedAuthenticationMiddleware', + # Answers 401 instead of a redirect to login when the client is not asking for HTML + 'cms.djangoapps.contentstore.middleware.LoginRedirectToUnauthorizedMiddleware', + 'common.djangoapps.student.middleware.UserStandingMiddleware', 'django.contrib.messages.middleware.MessageMiddleware',