feat: only redirect to login for top-level page navigation requests (#29054)

Commit modifies safe session middleware to return an 401 in case of authentication failure and lack of 'text/html' in Accept header.

Previously, the middleware would always redirect to login in case of auth failure, but this was deemed inappropriate for any requests that are not top-level page navigation requests(we check this by seeming if 'text/html' is precent in Accept header)

Co-authored-by: Robert Raposa <rraposa@edx.org>
This commit is contained in:
Manjinder Singh
2021-10-21 08:45:52 -04:00
committed by GitHub
parent 99ceec8647
commit ef135fba5a
2 changed files with 29 additions and 1 deletions

View File

@@ -394,7 +394,25 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin):
if is_request_from_mobile_app(request):
return HttpResponse(status=401)
return redirect_to_login(request.path)
# .. toggle_name: REDIRECT_TO_LOGIN_ON_SAFE_SESSION_AUTH_FAILURE
# .. toggle_implementation: SettingToggle
# .. toggle_default: True
# .. toggle_description: Turn this toggle off to roll out new functionality,
# which returns a 401 rather than redirecting to login, when HTML is not expected by the client.
# .. toggle_use_cases: temporary
# .. toggle_creation_date: 2021-10-18
# .. toggle_target_removal_date: 2021-10-22
# .. toggle_tickets: https://openedx.atlassian.net/browse/ARCHBOM-1911
REDIRECT_TO_LOGIN_ON_SAFE_SESSION_AUTH_FAILURE = getattr(settings,
'REDIRECT_TO_LOGIN_ON_SAFE_SESSION_AUTH_FAILURE',
True
)
if REDIRECT_TO_LOGIN_ON_SAFE_SESSION_AUTH_FAILURE or 'text/html' in request.META.get('HTTP_ACCEPT', ''):
set_custom_attribute("safe_sessions.auth_failure", "redirect_to_login")
return redirect_to_login(request.path)
set_custom_attribute("safe_sessions.auth_failure", "401")
return HttpResponse(status=401)
@staticmethod
def _verify_user(request, response, userid_in_session):

View File

@@ -123,6 +123,7 @@ class TestSafeSessionProcessRequest(TestSafeSessionsLogMixin, TestCase):
self.assert_no_user_in_session()
def test_parse_error_at_step_1(self):
self.request.META['HTTP_ACCEPT'] = 'text/html'
with self.assert_parse_error():
self.assert_response('not-a-safe-cookie', success=False)
self.assert_no_session()
@@ -130,6 +131,7 @@ class TestSafeSessionProcessRequest(TestSafeSessionsLogMixin, TestCase):
def test_invalid_user_at_step_4(self):
self.client.login(username=self.user.username, password='test')
safe_cookie_data = SafeCookieData.create(self.client.session.session_key, 'no_such_user')
self.request.META['HTTP_ACCEPT'] = 'text/html'
with self.assert_incorrect_user_logged():
self.assert_response(safe_cookie_data, success=False)
self.assert_user_in_session()
@@ -314,8 +316,16 @@ class TestSafeSessionMiddleware(TestSafeSessionsLogMixin, TestCase):
assert mock_delete_cookie.called
def test_error(self):
self.request.META['HTTP_ACCEPT'] = 'text/html'
self.verify_error(302)
@ddt.data(['text/html', 302], ['', 401])
@ddt.unpack
@override_settings(REDIRECT_TO_LOGIN_ON_SAFE_SESSION_AUTH_FAILURE=False)
def test_error_with_http_accept(self, http_accept, expected_response):
self.request.META['HTTP_ACCEPT'] = http_accept
self.verify_error(expected_response)
@override_settings(MOBILE_APP_USER_AGENT_REGEXES=[r'open edX Mobile App'])
def test_error_from_mobile_app(self):
self.request.META = {'HTTP_USER_AGENT': 'open edX Mobile App Version 2.1'}