fix!: session ID stability while maintaining inactivity timeout

openedx/core/djangoapps/session_inactivity_timeout/middleware.py

@@ -14,12 +14,18 @@
 
 
 from datetime import datetime, timedelta
+import logging
 
 from django.conf import settings
 from django.contrib import auth
 from django.utils.deprecation import MiddlewareMixin
+from edx_django_utils import monitoring as monitoring_utils
+from edx_django_utils.cache import TieredCache
 
-LAST_TOUCH_KEYNAME = 'SessionInactivityTimeout:last_touch'
+LAST_TOUCH_KEYNAME = 'SessionInactivityTimeout:last_touch_str'
+LAST_SESSION_SAVE_TIME_KEYNAME = 'SessionInactivityTimeout:last_session_save_time'
+
+log = logging.getLogger(__name__)
 
 
 class SessionInactivityTimeout(MiddlewareMixin):
@@ -34,27 +40,67 @@ def process_request(self, request):
             #Can't log out if not logged in
             return
 
+        # .. setting_name: SESSION_INACTIVITY_TIMEOUT_IN_SECONDS
+        # .. setting_default: None
+        # .. setting_description: If set, this is used to end the session when there is no activity for N seconds.
+        # .. setting_warning:  Keep in sync with SESSION_COOKIE_AGE and must be larger than SESSION_ACTIVITY_SAVE_DELAY_SECONDS.
         timeout_in_seconds = getattr(settings, "SESSION_INACTIVITY_TIMEOUT_IN_SECONDS", None)
 
         # Do we have this feature enabled?
         if timeout_in_seconds:
+            # .. setting_name: SESSION_ACTIVITY_SAVE_DELAY_SECONDS
+            # .. setting_default: 900 (15 minutes in seconds)
+            # .. setting_description: How often to allow a full session save (in seconds).
+            #   This controls how frequently the session ID might change.
+            #   A user could be inactive for almost SESSION_ACTIVITY_SAVE_DELAY_SECONDS but since their session 
+            #   isn't being saved during that time, their last activity timestamp isn't being updated.
+            #   When they hit the inactivity timeout, it will be based on the last saved activity time.
+            #   So the effective timeout could be as short as: SESSION_INACTIVITY_TIMEOUT_IN_SECONDS - SESSION_ACTIVITY_SAVE_DELAY_SECONDS.
+            #   This means users might be logged out earlier than expected in some edge cases.
+            # .. setting_warning:  Must be smaller than SESSION_INACTIVITY_TIMEOUT_IN_SECONDS.
+            frequency_time_in_seconds = getattr(settings, "SESSION_ACTIVITY_SAVE_DELAY_SECONDS", 900)
             # what time is it now?
             utc_now = datetime.utcnow()
 
             # Get the last time user made a request to server, which is stored in session data
-            last_touch = request.session.get(LAST_TOUCH_KEYNAME)
+            last_touch_str = request.session.get(LAST_TOUCH_KEYNAME)
 
             # have we stored a 'last visited' in session? NOTE: first time access after login
             # this key will not be present in the session data
-            if last_touch:
-                # compute the delta since last time user came to the server
-                time_since_last_activity = utc_now - last_touch
-
-                # did we exceed the timeout limit?
-                if time_since_last_activity > timedelta(seconds=timeout_in_seconds):
-                    # yes? Then log the user out
-                    del request.session[LAST_TOUCH_KEYNAME]
-                    auth.logout(request)
-                    return
-
-            request.session[LAST_TOUCH_KEYNAME] = utc_now
+            if last_touch_str:
+                try:
+                    last_touch = datetime.fromisoformat(last_touch_str)
+                    time_since_last_activity = utc_now - last_touch
+
+                    has_exceeded_timeout_limit = time_since_last_activity > timedelta(seconds=timeout_in_seconds)
+                    if has_exceeded_timeout_limit:
+                        del request.session[LAST_TOUCH_KEYNAME]
+                        auth.logout(request)
+                        return
+                except (ValueError, TypeError) as e:
+                    log.warning("Parsing last touch time failed: %s", e)
+                    # If parsing fails, treat as if no timestamp exists
+                    pass
+            else:
+                # .. custom_attribute_name: session_inactivity.first_login
+                # .. custom_attribute_description: Tracks when users have no stored activity
+                #   timestamp (first access after login).
+                monitoring_utils.set_custom_attribute('session_inactivity.first_login', True)
+                log.debug("No previous activity timestamp found (first login)")
+
+            cached_response = TieredCache.get_cached_response(LAST_SESSION_SAVE_TIME_KEYNAME)
+            last_save = cached_response.value if cached_response.is_found else None
+            current_time = datetime.utcnow().isoformat()
+
+            # .. custom_attribute_name: session_inactivity.activity_seen
+            # .. custom_attribute_description: Records the timestamp when user activity is detected.
+            monitoring_utils.set_custom_attribute('session_inactivity.activity_seen', current_time)
+            if not last_save or (
+                datetime.fromisoformat(last_save) + timedelta(seconds=frequency_time_in_seconds) < datetime.utcnow()
+            ):
+                # Allow a full session save periodically
+                TieredCache.set_all_tiers(LAST_SESSION_SAVE_TIME_KEYNAME, current_time, frequency_time_in_seconds)
+                # .. custom_attribute_name: session_inactivity.session_extended
+                # .. custom_attribute_description:  Marks when sessions are extended through the periodic save.
+                monitoring_utils.set_custom_attribute('session_inactivity.session_extended', True)
+                request.session[LAST_TOUCH_KEYNAME] = utc_now.isoformat()

openedx/core/djangoapps/session_inactivity_timeout/tests/__init__.py

@@ -0,0 +1,3 @@
+"""
+Tests for session inactivity timeout middleware.
+"""