security: Make sure that there is no trace of a member after they left

src/chat.rs

@@ -3990,6 +3990,30 @@ pub(crate) async fn remove_from_chat_contacts_table(
     Ok(())
 }
 
+/// Removes a contact from the chat
+/// without leaving a trace.
+///
+/// Note that if we receive a message
+/// from another device that doesn't know that this this member was removed
+/// then the group membership algorithm won't remember that this member was removed,
+/// so that the member will be wrongly re-added
+pub(crate) async fn remove_from_chat_contacts_table_without_trace(
+    context: &Context,
+    chat_id: ChatId,
+    contact_id: ContactId,
+) -> Result<()> {
+    context
+        .sql
+        .execute(
+            "DELETE FROM chats_contacts
+                WHERE chat_id=? AND contact_id=?",
+            (chat_id, contact_id),
+        )
+        .await?;
+
+    Ok(())
+}
+
 /// Adds a contact to the chat.
 /// If the group is promoted, also sends out a system message to all group members
 pub async fn add_contact_to_chat(
@@ -4265,14 +4289,7 @@ pub async fn remove_contact_from_chat(
             if chat.is_promoted() {
                 remove_from_chat_contacts_table(context, chat_id, contact_id).await?;
             } else {
-                context
+                remove_from_chat_contacts_table_without_trace(context, chat_id, contact_id).await?;
-                    .sql
-                    .execute(
-                        "DELETE FROM chats_contacts
-                         WHERE chat_id=? AND contact_id=?",
-                        (chat_id, contact_id),
-                    )
-                    .await?;
             }
 
             // We do not return an error if the contact does not exist in the database.

src/chat/chat_tests.rs

@@ -3090,7 +3090,12 @@ async fn test_leave_broadcast() -> Result<()> {
     let leave_msg = bob.pop_sent_msg().await;
     alice.recv_msg_trash(&leave_msg).await;
 
-    assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 0);
+    assert!(get_chat_contacts(alice, alice_chat_id).await?.is_empty());
+    assert!(
+        get_past_chat_contacts(alice, alice_chat_id)
+            .await?
+            .is_empty()
+    );
 
     alice.emit_event(EventType::Test);
     alice
@@ -4122,12 +4127,11 @@ async fn test_sync_broadcast() -> Result<()> {
     tcm.section("Alice's second device receives the removal-message");
     alice2.recv_msg(&sent).await;
     assert!(get_chat_contacts(alice2, a2_broadcast_id).await?.is_empty());
-    // TODO do we want to make sure that there is no trace of a member?
+    assert!(
-    // assert!(
+        get_past_chat_contacts(alice2, a2_broadcast_id)
-    //     get_past_chat_contacts(alice1, a1_broadcast_id)
+            .await?
-    //         .await?
+            .is_empty()
-    //         .is_empty()
+    );
-    // );
 
     tcm.section("Bob receives the removal-message");
     bob.recv_msg(&sent).await;

src/receive_imf.rs

@@ -3512,11 +3512,12 @@ async fn apply_out_broadcast_changes(
         if removed_id == Some(from_id) {
             // The sender of the message left the broadcast channel
             // Silently remove them without notifying the user
-            chat::remove_from_chat_contacts_table(context, chat.id, from_id).await?;
+            chat::remove_from_chat_contacts_table_without_trace(context, chat.id, from_id).await?;
             better_msg = Some("".to_string());
         } else if from_id == ContactId::SELF {
             if let Some(removed_id) = removed_id {
-                chat::remove_from_chat_contacts_table(context, chat.id, removed_id).await?;
+                chat::remove_from_chat_contacts_table_without_trace(context, chat.id, removed_id)
+                    .await?;
 
                 better_msg.get_or_insert(
                     stock_str::msg_del_member_local(context, removed_id, ContactId::SELF).await,