feat: Move unverified messages carrying current Autocrypt key from protected 1:1 chats to ad-hoc groups

TODO: This should be squashed with the previous commit if we decide to go this way.

src/receive_imf.rs

@@ -602,11 +602,7 @@ async fn add_parts(
                 context,
                 mime_parser,
                 is_partial_download.is_some(),
-                if test_normal_chat.is_none() {
+                test_normal_chat.is_some() || allow_creation,
-                    allow_creation
-                } else {
-                    true
-                },
                 create_blocked,
                 from_id,
                 to_ids,
@@ -618,6 +614,7 @@ async fn add_parts(
             }
         }
 
+        loop {
         // if the chat is somehow blocked but we want to create a non-blocked chat,
         // unblock the chat
         if chat_id_blocked != Blocked::Not && create_blocked != Blocked::Yes {
@@ -706,7 +703,7 @@ async fn add_parts(
                 }
             };
 
-            if let Some(chat) = test_normal_chat {
+            if let Some(chat) = test_normal_chat.as_ref() {
                 chat_id = Some(chat.id);
                 chat_id_blocked = chat.blocked;
             } else if allow_creation {
@@ -720,7 +717,8 @@ async fn add_parts(
                 }
             }
 
-            if let Some(chat_id) = chat_id {
+            let chat_id_ref = &mut chat_id;
+            if let Some(chat_id) = *chat_id_ref {
                 if chat_id_blocked != Blocked::Not {
                     if chat_id_blocked != create_blocked {
                         chat_id.set_blocked(context, create_blocked).await?;
@@ -776,7 +774,33 @@ async fn add_parts(
                                         .as_ref()
                                         .and_then(|p| p.verified_key_fingerprint.as_ref())
                             }) {
-                                Some(_) => chat.protected,
+                                Some(_) => {
+                                    if let Some(new_chat_id) = create_adhoc_group(
+                                        context,
+                                        mime_parser,
+                                        create_blocked,
+                                        &[ContactId::SELF, from_id],
+                                    )
+                                    .await
+                                    .context("could not create ad hoc group")?
+                                    {
+                                        info!(
+                                            context,
+                                            "Moving message to a new ad-hoc group keeping 1:1 chat \
+                                            protection.",
+                                        );
+                                        *chat_id_ref = Some(new_chat_id);
+                                        chat_id_blocked = create_blocked;
+                                        continue;
+                                    } else {
+                                        warn!(
+                                            context,
+                                            "Rejected to create an ad-hoc group, but keeping 1:1 chat \
+                                            protection.",
+                                        );
+                                    }
+                                    chat.protected
+                                }
                                 None => ProtectionStatus::ProtectionBroken,
                             };
                     }
@@ -807,6 +831,8 @@ async fn add_parts(
         } else {
             MessageState::InFresh
         };
+        break;
+        }
     } else {
         // Outgoing
 
@@ -1493,7 +1519,16 @@ async fn lookup_chat_by_reply(
 
     // If this was a private message just to self, it was probably a private reply.
     // It should not go into the group then, but into the private chat.
-    if is_probably_private_reply(context, to_ids, from_id, mime_parser, parent_chat.id).await? {
+    if is_probably_private_reply(
+        context,
+        to_ids,
+        from_id,
+        mime_parser,
+        parent_chat.id,
+        &parent_chat.grpid,
+    )
+    .await?
+    {
         return Ok(None);
     }
 
@@ -1523,13 +1558,14 @@ async fn is_probably_private_reply(
     from_id: ContactId,
     mime_parser: &MimeMessage,
     parent_chat_id: ChatId,
+    parent_chat_grpid: &str,
 ) -> Result<bool> {
     // Usually we don't want to show private replies in the parent chat, but in the
     // 1:1 chat with the sender.
     //
-    // There is one exception: Classical MUA replies to two-member groups
+    // An exception is replies to 2-member groups from classical MUAs or to 2-member ad-hoc
-    // should be assigned to the group chat. We restrict this exception to classical emails, as chat-group-messages
+    // groups. Such messages can't contain a Chat-Group-Id header and need to be sorted purely by
-    // contain a Chat-Group-Id header and can be sorted into the correct chat this way.
+    // References/In-Reply-To.
 
     let private_message =
         (to_ids == [ContactId::SELF]) || (from_id == ContactId::SELF && to_ids.len() == 1);
@@ -1537,7 +1573,7 @@ async fn is_probably_private_reply(
         return Ok(false);
     }
 
-    if !mime_parser.has_chat_version() {
+    if !mime_parser.has_chat_version() || parent_chat_grpid.is_empty() {
         let chat_contacts = chat::get_chat_contacts(context, parent_chat_id).await?;
         if chat_contacts.len() == 2 && chat_contacts.contains(&ContactId::SELF) {
             return Ok(false);
@@ -1572,6 +1608,10 @@ async fn create_or_lookup_group(
             member_ids.push(ContactId::SELF);
         }
 
+        if member_ids.len() < 3 {
+            info!(context, "Not creating ad-hoc group: too few contacts.");
+            return Ok(None);
+        }
         let res = create_adhoc_group(context, mime_parser, create_blocked, &member_ids)
             .await
             .context("could not create ad hoc group")?
@@ -1596,7 +1636,8 @@ async fn create_or_lookup_group(
     // they belong to the group because of the Chat-Group-Id or Message-Id header
     if let Some(chat_id) = chat_id {
         if !mime_parser.has_chat_version()
-            && is_probably_private_reply(context, to_ids, from_id, mime_parser, chat_id).await?
+            && is_probably_private_reply(context, to_ids, from_id, mime_parser, chat_id, &grpid)
+                .await?
         {
             return Ok(None);
         }
@@ -2225,11 +2266,6 @@ async fn create_adhoc_group(
         return Ok(None);
     }
 
-    if member_ids.len() < 3 {
-        info!(context, "Not creating ad-hoc group: too few contacts.");
-        return Ok(None);
-    }
-
     // use subject as initial chat name
     let grpname = mime_parser
         .get_subject()

src/test_utils.rs

@@ -136,22 +136,6 @@ impl TestContextManager {
         received_msg
     }
 
-    /// - Let one TestContext send a message.
-    /// - Let the other TestContext receive it.
-    /// - Assert that the message text is the formatted `err`.
-    /// - Assert that the message info contains the original text.
-    pub async fn send_recv_with_err(
-        &self,
-        from: &TestContext,
-        to: &TestContext,
-        err: &str,
-        msg: &str,
-    ) -> Message {
-        let received_msg = self.try_send_recv(from, to, msg).await;
-        check_msg_with_err(to, &received_msg, err, msg).await;
-        received_msg
-    }
-
     /// - Let one TestContext send a message
     /// - Let the other TestContext receive it
     pub async fn try_send_recv(&self, from: &TestContext, to: &TestContext, msg: &str) -> Message {
@@ -1214,18 +1198,6 @@ async fn write_msg(context: &Context, prefix: &str, msg: &Message, buf: &mut Str
     .unwrap();
 }
 
-/// - Assert that the message text is the formatted error `err`.
-/// - Assert that the message info contains the original text.
-pub async fn check_msg_with_err(ctx: &TestContext, msg: &Message, err: &str, text: &str) {
-    assert_eq!(msg.text, format!("[{err}]"));
-    assert!(msg
-        .id
-        .get_info(ctx)
-        .await
-        .unwrap()
-        .contains(&format!("\n\n{text}\n\n")));
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

src/tests/verified_chats.rs

@@ -1,10 +1,10 @@
 use anyhow::Result;
 use pretty_assertions::assert_eq;
 
-use crate::chat::{add_contact_to_chat, create_broadcast_list, Chat, ProtectionStatus};
+use crate::chat::{Chat, ProtectionStatus};
 use crate::chatlist::Chatlist;
 use crate::config::Config;
-use crate::constants::DC_GCL_FOR_FORWARDING;
+use crate::constants::{Chattype, DC_GCL_FOR_FORWARDING};
 use crate::contact::VerifiedStatus;
 use crate::contact::{Contact, Origin};
 use crate::message::{Message, Viewtype};
@@ -13,7 +13,7 @@ use crate::mimeparser::SystemMessage;
 use crate::receive_imf::receive_imf;
 use crate::stock_str;
 use crate::test_utils::{
-    check_msg_with_err, get_chat_msg, mark_as_verified, TestContext, TestContextManager,
+    get_chat_msg, mark_as_verified, TestContext, TestContextManager,
 };
 use crate::{e2ee, message};
 
@@ -745,6 +745,7 @@ async fn test_create_oneonone_chat_with_former_verified_contact() -> Result<()>
 }
 
 /// Some messages are sent unencrypted, but they mustn't break a verified chat protection.
+/// They must go to a new 2-member ad-hoc group instead.
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_unencrypted() -> Result<()> {
     let mut tcm = TestContextManager::new();
@@ -752,30 +753,34 @@ async fn test_unencrypted() -> Result<()> {
     let bob = tcm.bob().await;
     enable_verified_oneonone_chats(&[&alice]).await;
     mark_as_verified(&alice, &bob).await;
+    alice.create_chat(&bob).await;
 
-    let err_str = "This message is not encrypted. See 'Info' for more details";
+    let msg = tcm.send_recv(&bob, &alice, "hi").await;
-    let msg = tcm.send_recv_with_err(&bob, &alice, err_str, "hi").await;
     assert!(!msg.get_showpadlock());
     let alice_chat = Chat::load_from_db(&alice, msg.chat_id).await?;
-    assert!(alice_chat.is_protected());
+    assert_eq!(alice_chat.typ, Chattype::Group);
-    assert!(!alice_chat.is_protection_broken());
 
-    let broadcast_id = create_broadcast_list(&bob).await?;
+    // The next unencrypted message must get to the same ad-hoc group thanks to "In-Reply-To".
-    add_contact_to_chat(
+    let msg = tcm.send_recv(&bob, &alice, "hi again").await;
-        &bob,
-        broadcast_id,
-        bob.add_or_lookup_contact(&alice).await.id,
-    )
-    .await?;
-    let sent_msg = bob.send_text(broadcast_id, "hi all").await;
-    let msg = alice.recv_msg(&sent_msg).await;
-    check_msg_with_err(&alice, &msg, err_str, "hi all").await;
     assert!(!msg.get_showpadlock());
     assert_eq!(msg.chat_id, alice_chat.id);
     let alice_chat = Chat::load_from_db(&alice, msg.chat_id).await?;
+    assert_eq!(alice_chat.typ, Chattype::Group);
+
+    // This message is missed by Alice.
+    let chat_id = bob.get_chat(&alice).await.id;
+    bob.send_text(chat_id, "hi to the void").await;
+
+    // But the next message must get to the same ad-hoc group thanks to "References".
+    let msg = tcm.send_recv(&bob, &alice, "hi in a new group").await;
+    assert!(!msg.get_showpadlock());
+    assert_eq!(msg.chat_id, alice_chat.id);
+    let alice_chat = Chat::load_from_db(&alice, msg.chat_id).await?;
+    assert_eq!(alice_chat.typ, Chattype::Group);
+
+    let alice_chat = alice.get_chat(&bob).await;
     assert!(alice_chat.is_protected());
     assert!(!alice_chat.is_protection_broken());
-
     alice
         .golden_test_chat(alice_chat.id, "verified_chats_test_unencrypted")
         .await;

test-data/golden/verified_chats_test_unencrypted

@@ -1,6 +1,4 @@
 Single#Chat#10: bob@example.net [bob@example.net] 🛡️
 --------------------------------------------------------------------------------
 Msg#10: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️]
-Msg#11:  (Contact#Contact#10): [This message is not encrypted. See 'Info' for more details] [FRESH]
-Msg#12:  (Contact#Contact#10): [This message is not encrypted. See 'Info' for more details] [FRESH]
 --------------------------------------------------------------------------------