Merge commit 'refs/pull/499/head' of github.com:camptocamp/odoo-cloud-platform into merge-branch-3525-add-pending-module-19-380157c8

Author: cyrilmanuel

session_redis/__manifest__.py

@@ -5,7 +5,7 @@
 {
     "name": "Sessions in Redis",
     "summary": "Store web sessions in Redis",
-    "version": "18.0.1.0.0",
+    "version": "19.0.1.0.0",
     "author": "Camptocamp,Odoo Community Association (OCA)",
     "license": "AGPL-3",
     "category": "Extra Tools",

session_redis/http.py

@@ -1,12 +1,11 @@
 # Copyright 2016-2024 Camptocamp SA
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html)
-
+import functools
 import logging
 import os
 
 from odoo import http
 from odoo.tools import config
-from odoo.tools.func import lazy_property
 
 from .session import RedisSessionStore
 from .strtobool import strtobool
@@ -46,7 +45,7 @@ def is_true(strval):
 redis_cluster = os.getenv("ODOO_SESSION_REDIS_CLUSTER", "0")
 
 
-@lazy_property
+@functools.cached_property
 def session_store(self):
     if sentinel_host:
         sentinel = Sentinel([(sentinel_host, sentinel_port)], password=password)
@@ -108,5 +107,14 @@ def purge_fs_sessions(path):
             port,
         )
     http.Application.session_store = session_store
+    # cached_property needs __set_name__ to be called, but it is not called
+    # automatically since we are attaching the property after instance creation.
+    # So we have to do it manually
+    # See: https://docs.python.org/3/reference/datamodel.html#object.__set_name__
+    # Credit: https://stackoverflow.com/a/62161136
+    http.Application.session_store.__set_name__(
+        http.Application,
+        "session_store",
+    )
     # clean the existing sessions on the file system
     purge_fs_sessions(config.session_dir)

session_redis/session.py

@@ -3,20 +3,29 @@
 
 import json
 import logging
+from typing import TypeAlias, List
 
-from odoo.service import security
+import odoo.http
+from odoo.http import SESSION_LIFETIME
 from odoo.tools._vendor.sessions import SessionStore
 
 from . import json_encoding
 
-# this is equal to the duration of the session garbage collector in
+# this was equal to the duration of the session garbage collector in
 # odoo.http.session_gc()
-DEFAULT_SESSION_TIMEOUT = 60 * 60 * 24 * 7  # 7 days in seconds
 DEFAULT_SESSION_TIMEOUT_ANONYMOUS = 60 * 60 * 3  # 3 hours in seconds
 
 _logger = logging.getLogger(__name__)
 
 
+# Many parts of the session store API operate not on full session keys, but only
+# the first n characters of them (see odoo.http.STORED_SESSION_BYTES). In
+# particular used by Devices, but Odoo in general seems to promise that this
+# partial sid will be safe to store in the database, and can be used to later
+# find sessions, even if those sessions are actually longer.
+PartialSid: TypeAlias = str
+
+
 class RedisSessionStore(SessionStore):
     """SessionStore that saves session to redis"""
 
@@ -31,7 +40,7 @@ def __init__(
         super().__init__(session_class=session_class)
         self.redis = redis
         if expiration is None:
-            self.expiration = DEFAULT_SESSION_TIMEOUT
+            self.expiration = SESSION_LIFETIME
         else:
             self.expiration = expiration
         if anon_expiration is None:
@@ -42,18 +51,37 @@ def __init__(
         if prefix:
             self.prefix = f"{self.prefix}:{prefix}:"
 
+    # Use the key generation method of the FileSystemSessionStore: it seems that
+    # the one on the general SessionStore does not generate long enough keys to
+    # support the device session rotation logic (SessionStore produces 40
+    # character long keys, while the new rotation logic appears to assume a
+    # length of at least 84).
+    generate_key = odoo.http.FilesystemSessionStore.generate_key
+    is_valid_key = odoo.http.FilesystemSessionStore.is_valid_key
+
     def build_key(self, sid):
         return f"{self.prefix}{sid}"
 
     def save(self, session):
         key = self.build_key(session.sid)
 
-        # allow to set a custom expiration for a session
+        # If the session has a deletion_time, it is slated for rotation, and
+        # should be removed once the rotation window is over. See
+        # odoo.http.SESSION_DELETION_TIMER.
+        # Otherwise, allow to set a custom expiration for a session
         # such as a very short one for monitoring requests
         if session.uid:
-            expiration = session.expiration or self.expiration
+            expiration = (
+                session.get("deletion_time")
+                or session.get("expiration")
+                or self.expiration
+            )
         else:
-            expiration = session.expiration or self.anon_expiration
+            expiration = (
+                session.get("deletion_time")
+                or session.get("expiration")
+                or self.anon_expiration
+            )
         if _logger.isEnabledFor(logging.DEBUG):
             if session.uid:
                 user_msg = f"user '{session.login}' (id: {session.uid})"
@@ -106,12 +134,9 @@ def list(self):
         _logger.debug("a listing redis keys has been called")
         return [key[len(self.prefix) :] for key in keys]
 
-    def rotate(self, session, env):
-        self.delete(session)
-        session.sid = self.generate_key()
-        if session.uid and env:
-            session.session_token = security.compute_session_token(session, env)
-        self.save(session)
+    # The FilesystemSessionStore's rotate does not do anything file-system
+    # specific so it can just be reused here
+    rotate = odoo.http.FilesystemSessionStore.rotate
 
     def vacuum(self, *args, **kwargs):
         """Do not garbage collect the sessions
@@ -120,3 +145,62 @@ def vacuum(self, *args, **kwargs):
         expiration.
         """
         return None
+
+    def delete_old_sessions(self, session):
+        """
+        # Deletion of rotated sessions is handled by updating the sessions'
+        # expiry based on deletion_time in save(), so this method is redundant
+        # when using a redis store.
+
+        # While this method is not part of the generic SessionStore API, it is
+        # defined on the file session store, and is used by the Session itself
+        # as part of the session rotation (see odoo.http.Session._delete_old_sessions).
+        """
+        return
+
+    def get_missing_session_identifiers(self, identifiers: List[PartialSid]) -> set[PartialSid]:
+        """
+        Given a list of partial session ids, return a set of those session ids
+        which no longer exist in the keystore.
+
+        While this method is not part of the generic SessionStore API, it is
+        defined on the file session store, and is used by Odoo's devices to
+        figure out what needs to be revoked
+        (see odoo.addons.base.models.res_device.ResDeviceLog.__update_revoked).
+        """
+        identifiers = set(identifiers)
+        not_found = set()
+        for partial_sid in identifiers:
+            try:
+                next(
+                    self.redis.scan_iter(
+                        match=f"{self.prefix}{partial_sid}*",
+                        count=1,
+                    )
+                )
+            except StopIteration:
+                # No matches found
+                not_found.add(partial_sid)
+
+        return not_found
+
+    def delete_from_identifiers(self, identifiers: List[PartialSid]):
+        """
+        Given a list of partial session ids, remove any that are in the session store.
+
+        While this method is not part of the generic SessionStore API, it is
+        defined on the file session store, and is used by devices when revoking
+        device sessions (see odoo.addons.base.models.res_device.ResDevice._revoke).
+        """
+        patterns_to_unlink = []
+        for identifier in identifiers:
+            # Avoid removing a session if it does not match an identifier.
+            # See this same comment in odoo.http.FileSessionStore.delete_from_identifiers.
+            if not odoo.http._session_identifier_re.match(identifier):
+                raise ValueError("Identifier format incorrect, did you pass in a string instead of a list?")
+            patterns_to_unlink.append(f"{self.prefix}{identifier}*")
+        keys_to_unlink = []
+        for pattern in patterns_to_unlink:
+            keys_to_unlink.extend(self.redis.scan_iter(match=pattern))
+        if keys_to_unlink:
+            self.redis.delete(*keys_to_unlink)