From 02b9e059a2bf3631fdbc7d13f8e8ceb718d6ba52 Mon Sep 17 00:00:00 2001
From: Tim McCormack <tmccormack@edx.org>
Date: Thu, 2 Dec 2021 18:45:48 +0000
Subject: [PATCH] feat: Remove monitoring for now-concluded verify-all work
 (#29495)

The removed attributes were needed in order to inform the move of the
`_verify_user` function call up out of the try/except block. That work has
concluded (https://github.com/edx/edx-platform/pull/29324) so the
monitoring can be removed.

Also:

- Bring a comment on some other monitoring up to date
- Make long-needed corrections to an existing docstring
- Remove malformed-cookie logging, since we haven't been using it
---
 .../djangoapps/safe_sessions/middleware.py    | 46 ++++---------------
 1 file changed, 9 insertions(+), 37 deletions(-)

diff --git a/openedx/core/djangoapps/safe_sessions/middleware.py b/openedx/core/djangoapps/safe_sessions/middleware.py
index db6c634b7b..f94a268bc6 100644
--- a/openedx/core/djangoapps/safe_sessions/middleware.py
+++ b/openedx/core/djangoapps/safe_sessions/middleware.py
@@ -76,7 +76,6 @@ Custom Attributes:
 """
 
 import inspect
-from base64 import b64encode
 from hashlib import sha1, sha256
 from logging import getLogger
 from typing import Union
@@ -301,12 +300,14 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin):
         final verification before sending the response (in
         process_response).
         """
-        # 2021-10-29: Temporary debugging attr to answer the question
+        # 2021-12-01: Temporary debugging attr to answer the question
         # "are browsers sometimes sending in multiple session
-        # cookies?" We've observed behavior that might be consistent
-        # with this, perhaps due to an additional cookie set on the
-        # wrong domain, and assuming that the cookies *occasionally*
-        # are sent in a different order. -- timmc
+        # cookies?" Answer: Yes, this sometimes happens, although it
+        # does not appear to cause user mismatches. (There was a theory
+        # that multiple cookies might sometimes be sent in a varying
+        # order.) We may still want to have the ability to monitor this
+        # oddity, but as far as we can tell, it is not essential to the
+        # core safe-sessions monitoring.
         try:
             set_custom_attribute(
                 'safe_sessions.session_cookie_count',
@@ -358,12 +359,11 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin):
         cookie.
 
         Also, the session cookie is deleted if prior verification failed
-        or the designated user in the request has changed since the
-        original request.
+        or the new cookie can't be created for some reason.
 
         Processing the response is a multi-step process, as follows:
 
-        Step 1. Call the parent's method to generate the basic cookie.
+        Step 1. Call the parent's method to (maybe) generate the basic cookie.
 
         Step 2. Verify that the user marked at the time of
         process_request matches the user at this time when processing
@@ -378,26 +378,6 @@ class SafeSessionMiddleware(SessionMiddleware, MiddlewareMixin):
         """
         response = super().process_response(request, response)  # Step 1
 
-        # 2021-10-29: Temporary debugging attrs, to answer the
-        # question "are we calling _verify_user on too few responses?"
-        # We should probably be calling it on every response, and it
-        # looks like we might be missing most responses -- some
-        # testing shows that most of my LMS responses do *not* get a
-        # newly coined session cookie, but I seem to recall that that
-        # used to happen. If so, we may have at some point stopped
-        # calling _verify_user as often as we should. -- timmc
-        try:
-            set_custom_attribute(
-                'safe_sessions.request_had_valid_session',
-                hasattr(request, 'safe_cookie_verified_session_id')
-            )
-            set_custom_attribute(
-                'safe_sessions.response_has_session_cookie',
-                _is_cookie_present(response)
-            )
-        except:  # pylint: disable=bare-except
-            pass
-
         user_id_in_session = self.get_user_id_from_session(request)
         self._verify_user(request, response, user_id_in_session)  # Step 2
 
@@ -661,14 +641,6 @@ def _delete_cookie(request, response):
         httponly=settings.SESSION_COOKIE_HTTPONLY or None,
     )
 
-    # Log the cookie, but cap the length and base64 encode to make sure nothing
-    # malicious gets directly dumped into the log.
-    cookie_header = request.META.get('HTTP_COOKIE', '')[:4096]
-    log.warning(
-        "Malformed Cookie Header? First 4K, in Base64: %s",
-        b64encode(str(cookie_header).encode())
-    )
-
     # Note, there is no request.user attribute at this point.
     if hasattr(request, 'session') and hasattr(request.session, 'session_key'):
         log.warning(