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

feat: Enable ENFORCE_SAFE_SESSIONS by default; improve docs

Browse Source 
This toggle has been shown to work, so enable by default. Will need to be
documented in release notes for deployers.
This commit is contained in:
Tim McCormack
2022-01-15 00:56:31 +00:00
parent fb1ad76e65
commit 9827a077aa
2 changed files with 16 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
openedx/core/djangoapps/safe_sessions/middleware.py
View File

@@ -133,22 +133,24 @@ LOG_REQUEST_USER_CHANGE_HEADERS_DURATION = getattr(settings, 'LOG_REQUEST_USER_C
# .. toggle_name: ENFORCE_SAFE_SESSIONS
# .. toggle_implementation: SettingToggle
# .. toggle_default: False
# .. toggle_description: Turn this toggle on to enforce safe-sessions policy.
# .. toggle_default: True
# .. toggle_description: Invalidate session and response if mismatch detected.
# That is, when the `user` attribute of the request object gets changed or
# no longer matches the session, the session will be invalidated and the
# response cancelled (changed to an error). This is intended as a backup
# safety measure in case an attacker (or bug) is able to change the user
# on a session in an unexpected way. The behavior will be available for
# the Nutmeg named release and will become permanent in Olive.
# .. toggle_warnings: Before enabling, confirm that incidences of the string
# "SafeCookieData user at request" in the logs only show false positives,
# such as people logging in while in possession of an already-valid session
# cookie.
# on a session in an unexpected way.
# .. toggle_warnings: Should be disabled if debugging mismatches using the
# LOG_REQUEST_USER_CHANGE_HEADERS toggle, otherwise series of mismatching
# requests from the same user cannot be investigated. Additionally, if
# enabling for the first time, confirm that incidences of the string
# "SafeCookieData user at request" in the logs are very rare; if they are
# not, it is likely that there is either a bug or that a login or
# registration endpoint needs to call ``mark_user_change_as_expected``.
# .. toggle_use_cases: opt_out
# .. toggle_creation_date: 2021-12-01
# .. toggle_tickets: https://openedx.atlassian.net/browse/ARCHBOM-1861
ENFORCE_SAFE_SESSIONS = SettingToggle('ENFORCE_SAFE_SESSIONS', default=False)
ENFORCE_SAFE_SESSIONS = SettingToggle('ENFORCE_SAFE_SESSIONS', default=True)
log = getLogger(__name__)

8
openedx/core/djangoapps/safe_sessions/tests/test_middleware.py
View File

@@ -341,9 +341,10 @@ class TestSafeSessionMiddleware(TestSafeSessionsLogMixin, CacheIsolationTestCase
self.request.META = {'HTTP_USER_AGENT': 'open edX Mobile App Version 2.1'}
self.verify_error(401)
@override_settings(ENFORCE_SAFE_SESSIONS=False)
def test_warn_on_user_change_before_response(self):
"""
Verifies that warnings are emitted and custom attributes set if
Verifies that when enforcement disabled, warnings are emitted and custom attributes set if
the user changes unexpectedly between request and response.
"""
self.set_up_for_success()
@@ -358,10 +359,9 @@ class TestSafeSessionMiddleware(TestSafeSessionsLogMixin, CacheIsolationTestCase
set_attr_call_args = [call.args for call in mock_attr.call_args_list]
assert ("safe_sessions.user_mismatch", "request-response-mismatch") in set_attr_call_args
@override_settings(ENFORCE_SAFE_SESSIONS=True)
def test_enforce_on_user_change_before_response(self):
"""
Copy of test_warn_on_user_change_before_response but with enforcement enabled.
Copy of test_warn_on_user_change_before_response but with enforcement enabled (default).
The differences should be the status code and the session deletion.
"""
self.set_up_for_success()
@@ -379,6 +379,7 @@ class TestSafeSessionMiddleware(TestSafeSessionsLogMixin, CacheIsolationTestCase
set_attr_call_args = [call.args for call in mock_attr.call_args_list]
assert ("safe_sessions.user_mismatch", "request-response-mismatch") in set_attr_call_args
@override_settings(ENFORCE_SAFE_SESSIONS=False)
def test_warn_on_user_change_from_session(self):
"""
Verifies that warnings are emitted and custom attributes set if
@@ -395,6 +396,7 @@ class TestSafeSessionMiddleware(TestSafeSessionsLogMixin, CacheIsolationTestCase
set_attr_call_args = [call.args for call in mock_attr.call_args_list]
assert ("safe_sessions.user_mismatch", "request-session-mismatch") in set_attr_call_args
@override_settings(ENFORCE_SAFE_SESSIONS=False)
def test_warn_on_user_change_in_both(self):
"""
Verifies that warnings are emitted and custom attributes set if