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

feat!: Replace logging WaffleSwitch with a django settinge.

Browse Source 
This was initially introduced as a temporary flag to be able to get more
information.  But if we get this kind of issue again, we'll need
something like this logging to determine the source of the session
collision.  Rather than removing the code and adding it back in later,
convert this temporary switch into an opt-in setting that can be used
again in the future.

BREAKING_CHANGE: 'safe_session.log_request_user_changes' switch no
longer exists and is replaced with the 'LOG_REQUEST_USER_CHANGES' django
setting which defaults to 'False'
This commit is contained in:
Feanil Patel
2021-04-02 13:43:50 -04:00
parent 4f725aa152
commit 455033458c
2 changed files with 9 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -76,21 +76,19 @@ from django.utils.crypto import get_random_string
from django.utils.deprecation import MiddlewareMixin
from django.utils.encoding import python_2_unicode_compatible
from edx_django_utils.monitoring import set_custom_attribute
from edx_toggles.toggles import WaffleSwitch
from openedx.core.lib.mobile_utils import is_request_from_mobile_app
# .. toggle_name: safe_session.log_request_user_changes
# .. toggle_implementation: WaffleSwitch
# .. toggle_name: LOG_REQUEST_USER_CHANGES
# .. toggle_implementation: SettingToggle
# .. toggle_default: False
# .. toggle_description: Turn this toggle on to log anytime the `user` attribute of the request object gets
# changed. This will also log the location where the change is coming from to quickly find issues.
# .. toggle_warnings: This logging will be very verbose and so should probably not be left on all the time.
# .. toggle_use_cases: temporary
# .. toggle_use_cases: opt_in
# .. toggle_creation_date: 2021-03-25
# .. toggle_target_removal_date: 2021-05-01
# .. toggle_tickets: https://openedx.atlassian.net/browse/ARCHBOM-1718
LOG_REQUEST_USER_CHANGES_FLAG = WaffleSwitch('safe_session.log_request_user_changes', __name__)
LOG_REQUEST_USER_CHANGES = getattr(settings, 'LOG_REQUEST_USER_CHANGES', False)
log = getLogger(__name__)
@@ -308,7 +306,7 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin):
user_id = self.get_user_id_from_session(request)
if safe_cookie_data.verify(user_id): # Step 4
request.safe_cookie_verified_user_id = user_id # Step 5
if LOG_REQUEST_USER_CHANGES_FLAG.is_enabled():
if LOG_REQUEST_USER_CHANGES:
log_request_user_changes(request)
else:
return self._on_user_authentication_failed(request)

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

@@ -71,10 +71,9 @@ class TestSafeSessionProcessRequest(TestSafeSessionsLogMixin, TestCase):
"""
assert SafeSessionMiddleware.get_user_id_from_session(self.request) == self.user.id
@patch("openedx.core.djangoapps.safe_sessions.middleware.LOG_REQUEST_USER_CHANGES", False)
@patch("openedx.core.djangoapps.safe_sessions.middleware.log_request_user_changes")
@patch("openedx.core.djangoapps.safe_sessions.middleware.LOG_REQUEST_USER_CHANGES_FLAG")
def test_success(self, mock_log_request_user_changes_flag, mock_log_request_user_changes):
mock_log_request_user_changes_flag.is_enabled.return_value = False
def test_success(self, mock_log_request_user_changes):
self.client.login(username=self.user.username, password='test')
session_id = self.client.session.session_key
safe_cookie_data = SafeCookieData.create(session_id, self.user.id)
@@ -99,10 +98,9 @@ class TestSafeSessionProcessRequest(TestSafeSessionsLogMixin, TestCase):
# verify extra request_user_logging not called.
assert not mock_log_request_user_changes.called
@patch("openedx.core.djangoapps.safe_sessions.middleware.LOG_REQUEST_USER_CHANGES", True)
@patch("openedx.core.djangoapps.safe_sessions.middleware.log_request_user_changes")
@patch("openedx.core.djangoapps.safe_sessions.middleware.LOG_REQUEST_USER_CHANGES_FLAG")
def test_log_request_user_flag_on(self, mock_log_request_user_changes_flag, mock_log_request_user_changes):
mock_log_request_user_changes_flag.is_enabled.return_value = True
def test_log_request_user_on(self, mock_log_request_user_changes):
self.client.login(username=self.user.username, password='test')
session_id = self.client.session.session_key
safe_cookie_data = SafeCookieData.create(session_id, self.user.id)