fix session gc not removing locks (#2777)

SDESK-7458

apps/auth/service.py

@@ -85,13 +85,17 @@ def on_deleted(self, doc):
         :param doc: A deleted auth doc AKA a session
         :return:
         """
-        # Clear the session data when session has ended
-        flask.session.pop("session_token", None)
+        is_last_session = False
+        if flask.request:
+            # Clear the session data when session has ended
+            flask.session.pop("session_token", None)
+            sessions = self.get(req=None, lookup={"user": doc["user"]})
+            is_last_session = not sessions.count()
 
         # notify that the session has ended
-        sessions = self.get(req=None, lookup={"user": doc["user"]})
-        app.on_session_end(doc["user"], doc["_id"], is_last_session=not sessions.count())
-        self.set_user_last_activity(doc["user"], done=True)
+        app.on_session_end(doc["user"], doc["_id"], is_last_session=is_last_session)
+        if is_last_session:
+            self.set_user_last_activity(doc["user"], done=True)
 
     def is_authorized(self, **kwargs) -> bool:
         """

apps/auth/session_purge.py

@@ -40,9 +40,9 @@ def remove_expired_sessions(self):
         expiration_time = utcnow() - timedelta(minutes=expiry_minutes)
         logger.info("Deleting session not updated since {}".format(expiration_time))
         query = {"_updated": {"$lte": date_to_str(expiration_time)}}
-        sessions = auth_service.get(req=None, lookup=query)
-        for session in sessions:
-            auth_service.delete({"_id": str(session["_id"])})
+        sessions = list(auth_service.get(req=None, lookup=query))
+        if sessions:
+            auth_service.delete_docs(sessions)
         self._update_online_users()
 
     def _update_online_users(self):

apps/item_lock/components/item_lock.py

@@ -122,7 +122,7 @@ def lock(self, item_filter, user_id, session_id, action):
             # unlock the lock :)
             unlock(lock_id, remove=True)
 
-    def unlock(self, item_filter, user_id, session_id, etag):
+    def unlock(self, item_filter, user_id, session_id, etag, force=False):
         item_model = get_model(ItemModel)
         item = item_model.find_one(item_filter)
 
@@ -134,7 +134,7 @@ def unlock(self, item_filter, user_id, session_id, etag):
 
         can_user_unlock, error_message = self.can_unlock(item, user_id)
 
-        if can_user_unlock:
+        if can_user_unlock or force:
             self.app.on_item_unlock(item, user_id)
             updates = {}
 
@@ -172,10 +172,11 @@ def unlock(self, item_filter, user_id, session_id, etag):
 
     def unlock_session(self, user_id, session_id, is_last_session):
         item_model = get_model(ItemModel)
-        items = item_model.find({LOCK_SESSION: str(session_id)} if not is_last_session else {LOCK_USER: str(user_id)})
+        lookup = {LOCK_SESSION: str(session_id)} if not is_last_session else {LOCK_USER: str(user_id)}
+        items = item_model.find(lookup)
 
         for item in items:
-            self.unlock({"_id": item["_id"]}, user_id, session_id, None)
+            self.unlock({"_id": item["_id"]}, user_id, session_id, None, force=True)
 
     def can_lock(self, item, user_id, session_id):
         """

tests/auth/auth_test.py

@@ -10,6 +10,8 @@
 
 
 class AuthTestCase(TestCase):
+    test_context = False  # avoid request context
+
     def test_remove_expired_sessions_syncs_online_users(self):
         sess_id = ObjectId()
         user_ids = self.app.data.insert(
@@ -20,17 +22,28 @@ def test_remove_expired_sessions_syncs_online_users(self):
             ],
         )
         self.assertEqual(2, len(user_ids))
+        sessions = [
+            {"user": user_ids[0], "_updated": utcnow() - timedelta(days=50)},
+            {"user": user_ids[1], "_updated": utcnow()},
+            {"_id": sess_id, "user": user_ids[1], "_updated": utcnow()},
+        ]
+
+        self.app.data.insert("auth", sessions)
+        _ids = [_auth["user"] for _auth in self.app.data.find_all("auth")]
+        self.assertEqual(3, len(_ids))
 
         self.app.data.insert(
-            "auth",
+            "archive",
             [
-                {"user": user_ids[0], "_updated": utcnow() - timedelta(days=50)},
-                {"user": user_ids[1], "_updated": utcnow()},
-                {"_id": sess_id, "user": user_ids[1], "_updated": utcnow()},
+                {
+                    "_id": "locked-item",
+                    "lock_session": str(sessions[0]["_id"]),
+                    "lock_user": str(user_ids[0]),
+                    "state": "in_progress",
+                    "task": {"desk": ObjectId()},
+                }
             ],
         )
-        _ids = [_auth["user"] for _auth in self.app.data.find_all("auth")]
-        self.assertEqual(3, len(_ids))
 
         RemoveExpiredSessions().run()
 
@@ -43,6 +56,9 @@ def test_remove_expired_sessions_syncs_online_users(self):
         self.assertEqual(2, len(_ids))
         self.assertEqual(user_ids[1], _ids[0])
         self.assertEqual(user_ids[1], _ids[1])
+        items = self.app.data.find_all("archive")
+        assert items[0]["lock_user"] is None
+        assert items[0]["lock_session"] is None
 
     def test_is_current_user_admin(self):
         with patch("apps.auth.get_user", return_value={}):