Re-associate draft replies with "deleted" user account

securedrop_client/api_jobs/sync.py

@@ -1,11 +1,12 @@
 import logging
-from typing import Any, List
+from typing import Any, List, Optional
 
 from sdclientapi import API
+from sdclientapi import User as SDKUser
 from sqlalchemy.orm.session import Session
 
 from securedrop_client.api_jobs.base import ApiJob
-from securedrop_client.db import User
+from securedrop_client.db import DeletedUser, DraftReply, User
 from securedrop_client.storage import get_remote_data, update_local_storage
 
 logger = logging.getLogger(__name__)
@@ -43,25 +44,39 @@ def call_api(self, api_client: API, session: Session) -> Any:
         sources, submissions, replies = get_remote_data(api_client)
         update_local_storage(session, sources, submissions, replies, self.data_dir)
 
-    def _update_users(session: Session, remote_users: List[User]) -> None:
+    def _update_users(session: Session, remote_users: List[SDKUser]) -> None:
         """
         1. Create local user accounts for each remote user that doesn't already exist
         2. Update existing local users
-        3. Delete all remaining local user accounts that no longer exist on the server
+        3. Re-associate any draft replies sent by a user that is about to be deleted
+        4. Delete all remaining local user accounts that no longer exist on the server
         """
+        deleted_user_id = None  # type: Optional[int]
         local_users = {user.uuid: user for user in session.query(User).all()}
         for remote_user in remote_users:
             local_user = local_users.get(remote_user.uuid)
-            if not local_user:
+            if not local_user:  # Create local user account
                 new_user = User(
                     uuid=remote_user.uuid,
                     username=remote_user.username,
                     firstname=remote_user.first_name,
                     lastname=remote_user.last_name,
                 )
                 session.add(new_user)
-                logger.debug(f"Adding account for user {new_user.uuid}")
-            else:
+
+                # If the new user is the "deleted" user account, store its id in case we need to
+                # reassociate draft replies later.
+                if new_user.deleted:
+                    session.commit()
+                    deleted_user_id = new_user.id
+
+                logger.debug(f"Adding account for user with uuid='{new_user.uuid}'")
+            else:  # Update existing local users
+                # If the local user is the "deleted" user account, store its id in case we need to
+                # reassociate draft replies later.
+                if local_user.deleted:
+                    deleted_user_id = local_user.id
+
                 if local_user.username != remote_user.username:
                     local_user.username = remote_user.username
                 if local_user.firstname != remote_user.first_name:
@@ -70,8 +85,40 @@ def _update_users(session: Session, remote_users: List[User]) -> None:
                     local_user.lastname = remote_user.last_name
                 del local_users[remote_user.uuid]
 
+        # Delete all remaining local user accounts that no longer exist on the server.
+        #
+        # In order to support an edge case that can occur on a pre-2.2.0 server that does not create
+        # a "deleted" user account, the client will create one locally when there are draft replies
+        # that need to be re-assoicated. Once the "deleted" user account exists on the server, it
+        # will replace the local one.
         for uuid, account in local_users.items():
+            # Do not delete the local "deleted" user account if there is no "deleted" user account
+            # on the server.
+            if account.deleted and not deleted_user_id:
+                continue
+
+            # Get draft replies sent by the user who's account is about to be deleted.
+            draft_replies = session.query(DraftReply).filter_by(journalist_id=account.id).all()
+
+            # Create a local "deleted" user account if there is no "deleted" user account locally or
+            # on the server and we are about to delete a user.
+            if draft_replies and not account.deleted and not deleted_user_id:
+                deleted_user = DeletedUser()
+                session.add(deleted_user)
+                session.commit()  # commit so that we can retrieve the generated `id`
+                deleted_user_id = deleted_user.id
+                logger.debug(f"Creating DeletedUser with uuid='{deleted_user.uuid}'")
+
+            # Re-associate draft replies
+            for reply in draft_replies:
+                reply.journalist_id = deleted_user_id
+                logger.debug(f"DraftReply with uuid='{reply.uuid}' re-associated to DeletedUser")
+
+            # Ensure re-associated draft replies are committed to the db before deleting the account
+            if draft_replies:
+                session.commit()
+
             session.delete(account)
-            logger.debug(f"Deleting account for user {uuid}")
+            logger.debug(f"Deleting account for user with uuid='{uuid}'")
 
         session.commit()

securedrop_client/db.py

@@ -3,6 +3,7 @@
 from enum import Enum
 from pathlib import Path
 from typing import Any, List, Union  # noqa: F401
+from uuid import uuid4
 
 from sqlalchemy import (
     Boolean,
@@ -573,6 +574,11 @@ def initials(self) -> str:
             return self.username[0:2].lower()  # username must be at least 3 characters
 
 
+class DeletedUser(User):
+    def __init__(self) -> None:
+        super().__init__(uuid=str(uuid4()), username="deleted")
+
+
 class SeenFile(Base):
     __tablename__ = "seen_files"
     __table_args__ = (UniqueConstraint("file_id", "journalist_id"),)

securedrop_client/gui/widgets.py

@@ -2034,11 +2034,14 @@ def _on_update_authenticated_user(self, user: User) -> None:
         """
         When the user logs in or updates user info, update the reply badge.
         """
-        if user.uuid == self.sender.uuid:
-            self.sender_is_current_user = True
-            self.sender = user
-            self.sender_icon.setToolTip(self.sender.fullname)
-            self._update_styles()
+        try:
+            if self.sender and self.sender.uuid == user.uuid:
+                self.sender_is_current_user = True
+                self.sender = user
+                self.sender_icon.setToolTip(self.sender.fullname)
+                self._update_styles()
+        except sqlalchemy.orm.exc.ObjectDeletedError:
+            logger.debug("The sender was deleted.")
 
     @pyqtSlot(str, str, str)
     def _on_reply_success(self, source_uuid: str, uuid: str, content: str) -> None:

securedrop_client/storage.py

@@ -37,6 +37,7 @@
 from sqlalchemy.orm.session import Session
 
 from securedrop_client.db import (
+    DeletedUser,
     DraftReply,
     File,
     Message,
@@ -409,38 +410,33 @@ def update_replies(
     * New replies have an entry created in the local database.
     * Local replies not returned in the remote replies are deleted from the
       local database unless they are pending or failed.
-
-    If a reply references a new journalist username, add them to the database
-    as a new user.
     """
     local_replies_by_uuid = {r.uuid: r for r in local_replies}
-    users: Dict[str, User] = {}
+    deleted_user = session.query(User).filter_by(username="deleted").one_or_none()
+    user_cache: Dict[str, User] = {}
     source_cache = SourceCache(session)
     for reply in remote_replies:
-        user = users.get(reply.journalist_uuid)
-
+        user = user_cache.get(reply.journalist_uuid)
         if not user:
-            user = create_or_update_user(
-                reply.journalist_uuid,
-                reply.journalist_username,
-                reply.journalist_first_name,
-                reply.journalist_last_name,
-                session,
-            )
-            users[reply.journalist_uuid] = user
-        elif (
-            user.username != reply.journalist_username
-            or user.firstname != reply.journalist_first_name
-            or user.lastname != reply.journalist_last_name
-        ):
-            user = create_or_update_user(
-                reply.journalist_uuid,
-                reply.journalist_username,
-                reply.journalist_first_name,
-                reply.journalist_last_name,
-                session,
-            )
-            users[reply.journalist_uuid] = user
+            user = session.query(User).filter_by(uuid=reply.journalist_uuid).one_or_none()
+
+            # If the account for the sender does not exist, then replies will need to be associated
+            # to a local "deleted" user account.
+            #
+            # Once support for the pre-2.2.0 server is deprecated, this code can be removed, and the
+            # client can rely entirely on the /users endpoint to manage user accounts. Until then,
+            # we must handle the case where the pre-2.2.0 server returns a `journalist_uuid` of
+            # "deleted" for a reply's sender when no actual account exists with that uuid.
+            if not user:
+                user = deleted_user
+                if not user:
+                    user = DeletedUser()
+                    session.add(user)
+                    session.commit()
+                    deleted_user = user
+
+            # Add the retrieved or newly created "deleted" user to the cache
+            user_cache[reply.journalist_uuid] = user
 
         local_reply = local_replies_by_uuid.get(reply.uuid)
         if local_reply:

tests/api_jobs/test_sync.py

@@ -67,6 +67,165 @@ def test_MetadataSyncJob_deletes_user(mocker, homedir, session, session_maker):
     assert not local_user
 
 
+def test_MetadataSyncJob_does_not_delete_reserved_deleted_user(mocker, homedir, session):
+    """
+    Ensure that we do not delete the local "deleted" user account unless a "deleted" user account
+    exists on the server that we can replace it with.
+
+    This test is to ensure that we support an edge case that can occur on a pre-2.2.0 server
+    (before the server added support for creating an actual deleted user account).
+    """
+    api_client = mocker.patch("securedrop_client.logic.sdclientapi.API")
+    api_client.get_users = mocker.MagicMock(return_value=[])
+
+    reserved_deleted_user = factory.User(username="deleted")
+    session.add(reserved_deleted_user)
+
+    job = MetadataSyncJob(homedir)
+    job.call_api(api_client, session)
+
+    assert session.query(User).filter_by(username="deleted").one_or_none()
+
+
+def test_MetadataSyncJob_reassociates_draftreplies_to_new_account_before_deleting_user(
+    mocker, homedir, session
+):
+    """
+    Ensure that any draft replies sent by a user that is about to be deleted are re-associated
+    to a new "deleted" user account that exists on the server.
+    """
+    user_to_delete_with_drafts = factory.User()
+    session.add(user_to_delete_with_drafts)
+    session.commit()
+    draftreply = factory.DraftReply(journalist_id=user_to_delete_with_drafts.id)
+    session.add(draftreply)
+    session.commit()
+
+    remote_reserved_deleted_user = factory.RemoteUser(username="deleted")
+    # Set up get_users so that `user_to_delete_with_drafts` will be deleted and
+    # `remote_reserved_deleted_user` will be created since it exists on the server
+    api_client = mocker.patch("securedrop_client.logic.sdclientapi.API")
+    api_client.get_users = mocker.MagicMock(return_value=[remote_reserved_deleted_user])
+
+    job = MetadataSyncJob(homedir)
+    job.call_api(api_client, session)
+
+    # Ensure the account was deleted
+    deleted_user = session.query(User).filter_by(uuid=user_to_delete_with_drafts.uuid).one_or_none()
+    assert not deleted_user
+    # Ensure the "deleted" user account that exists on the server was created locally
+    reserved_deleted_user = session.query(User).filter_by(username="deleted").one_or_none()
+    assert reserved_deleted_user
+    assert reserved_deleted_user.uuid == remote_reserved_deleted_user.uuid
+    # Ensure draft replies are reassociated
+    assert draftreply.journalist_id == reserved_deleted_user.id
+
+
+def test_MetadataSyncJob_reassociates_draftreplies_to_existing_account_before_deleting_user(
+    mocker, homedir, session
+):
+    """
+    Ensure that any draft replies sent by a user that is about to be deleted are re-associated
+    to an existing "deleted" user account.
+    """
+    user_to_delete_with_drafts = factory.User()
+    session.add(user_to_delete_with_drafts)
+    session.commit()
+    draftreply = factory.DraftReply(journalist_id=user_to_delete_with_drafts.id)
+    session.add(draftreply)
+    session.commit()
+
+    local_reserved_deleted_user = factory.User(username="deleted")
+    session.add(local_reserved_deleted_user)
+    session.commit()
+
+    remote_reserved_deleted_user = factory.RemoteUser(
+        username="deleted", uuid=local_reserved_deleted_user.uuid
+    )
+    # Set up get_users so that `user_to_delete_with_drafts` will be deleted and
+    # `remote_reserved_deleted_user` will replace `local_reserved_deleted_user`
+    api_client = mocker.patch("securedrop_client.logic.sdclientapi.API")
+    api_client.get_users = mocker.MagicMock(return_value=[remote_reserved_deleted_user])
+
+    job = MetadataSyncJob(homedir)
+    job.call_api(api_client, session)
+
+    # Ensure the account was deleted
+    deleted_user = session.query(User).filter_by(uuid=user_to_delete_with_drafts.uuid).one_or_none()
+    assert not deleted_user
+    # Ensure the "deleted" user account still exists
+    reserved_deleted_user = session.query(User).filter_by(username="deleted").one_or_none()
+    assert reserved_deleted_user
+    assert reserved_deleted_user.uuid == local_reserved_deleted_user.uuid
+    # Ensure draft replies are reassociated
+    assert draftreply.journalist_id == reserved_deleted_user.id
+
+
+def test_MetadataSyncJob_reassociates_draftreplies_to_new_local_account_before_deleting_user(
+    mocker, homedir, session
+):
+    """
+    Ensure that any draft replies, sent by a user that is about to be deleted, are re-associated
+    to a new "deleted" user account that only exists locally.
+
+    This test is to ensure that we support an edge case that can occur on a pre-2.2.0 server
+    (before the server added support for creating an actual deleted user account).
+    """
+    user_to_delete_with_drafts = factory.User()
+    session.add(user_to_delete_with_drafts)
+    session.commit()
+
+    draftreply = factory.DraftReply(journalist_id=user_to_delete_with_drafts.id)
+    session.add(draftreply)
+    # Set up get_users so that `user_to_delete_with_drafts` will be deleted
+    api_client = mocker.patch("securedrop_client.logic.sdclientapi.API")
+    api_client.get_users = mocker.MagicMock(return_value=[])
+
+    job = MetadataSyncJob(homedir)
+    job.call_api(api_client, session)
+
+    # Ensure the account was deleted
+    deleted_user = session.query(User).filter_by(uuid=user_to_delete_with_drafts.uuid).one_or_none()
+    assert not deleted_user
+    # Ensure the "deleted" user account exists
+    reserved_deleted_user = session.query(User).filter_by(username="deleted").one_or_none()
+    assert reserved_deleted_user
+    # Ensure draft replies are reassociated
+    assert draftreply.journalist_id == reserved_deleted_user.id
+
+
+def test_MetadataSyncJob_replaces_reserved_deleted_user_account(mocker, homedir, session):
+    """
+    Ensure that we delete the local "deleted" user account if it is being replaced by a new account
+    from the server and that any draft replies are re-associated appropriately.
+
+    This test is to ensure that we support an edge case that can occur on a pre-2.2.0 server
+    (before the server added support for creating an actual deleted user account).
+    """
+    local_reserved_deleted_user = factory.User(username="deleted")
+    session.add(local_reserved_deleted_user)
+    session.commit()
+    draftreply = factory.DraftReply(journalist_id=local_reserved_deleted_user.id)
+    session.add(draftreply)
+    session.commit()
+
+    remote_reserved_deleted_user = factory.RemoteUser(username="deleted")
+    # Set up get_users so that `user_to_delete_with_drafts` will be deleted and
+    # `remote_reserved_deleted_user` will replace `local_reserved_deleted_user`
+    api_client = mocker.patch("securedrop_client.logic.sdclientapi.API")
+    api_client.get_users = mocker.MagicMock(return_value=[remote_reserved_deleted_user])
+
+    job = MetadataSyncJob(homedir)
+    job.call_api(api_client, session)
+
+    # Ensure `remote_reserved_deleted_user` replaced `local_reserved_deleted_user` and that the
+    # draft reply was reassociated to the new "deleted" user account.
+    reserved_deleted_user = session.query(User).filter_by(username="deleted").one_or_none()
+    assert reserved_deleted_user
+    assert reserved_deleted_user.uuid == remote_reserved_deleted_user.uuid
+    assert draftreply.journalist_id == reserved_deleted_user.id
+
+
 def test_MetadataSyncJob_success(mocker, homedir, session, session_maker):
     job = MetadataSyncJob(homedir)