fix: answer 401 instead of a login redirect to non-HTML Studio requests

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())

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)

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

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):
         """

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',