From 0ea1e935679b2fbd35ff80c9b5883adf22a6d0fa Mon Sep 17 00:00:00 2001 From: iequidoo Date: Thu, 7 Sep 2023 23:13:22 -0300 Subject: [PATCH] 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 | 74 ++++++++++++++----- src/test_utils.rs | 28 ------- src/tests/verified_chats.rs | 41 +++++----- .../golden/verified_chats_test_unencrypted | 2 - 4 files changed, 78 insertions(+), 67 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 309e72687..dc5dc9ec5 100644 --- a/src/receive_imf.rs +++ b/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() { - allow_creation - } else { - true - }, + test_normal_chat.is_some() || allow_creation, 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 { // 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 - // should be assigned to the group chat. We restrict this exception to classical emails, as chat-group-messages - // contain a Chat-Group-Id header and can be sorted into the correct chat this way. + // An exception is replies to 2-member groups from classical MUAs or to 2-member ad-hoc + // groups. Such messages can't contain a Chat-Group-Id header and need to be sorted purely by + // 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() diff --git a/src/test_utils.rs b/src/test_utils.rs index 98fbea15d..fe02cfa1f 100644 --- a/src/test_utils.rs +++ b/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::*; diff --git a/src/tests/verified_chats.rs b/src/tests/verified_chats.rs index 19345a393..fefea4f8c 100644 --- a/src/tests/verified_chats.rs +++ b/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_with_err(&bob, &alice, err_str, "hi").await; + let msg = tcm.send_recv(&bob, &alice, "hi").await; assert!(!msg.get_showpadlock()); let alice_chat = Chat::load_from_db(&alice, msg.chat_id).await?; - assert!(alice_chat.is_protected()); - assert!(!alice_chat.is_protection_broken()); + assert_eq!(alice_chat.typ, Chattype::Group); - let broadcast_id = create_broadcast_list(&bob).await?; - add_contact_to_chat( - &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; + // The next unencrypted message must get to the same ad-hoc group thanks to "In-Reply-To". + let msg = tcm.send_recv(&bob, &alice, "hi again").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; diff --git a/test-data/golden/verified_chats_test_unencrypted b/test-data/golden/verified_chats_test_unencrypted index 6c4608ad3..b837162ca 100644 --- a/test-data/golden/verified_chats_test_unencrypted +++ b/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] --------------------------------------------------------------------------------