From 551555180e2f7f754d74136682cb51bf281c60ce Mon Sep 17 00:00:00 2001 From: iequidoo Date: Mon, 23 Jun 2025 17:58:39 -0300 Subject: [PATCH] feat: Don't send SystemMessage::Member* in unencrypted groups Unencrypted groups are usually ones with other MUAs which don't understand Delta Chat system messages and show them as weird emails. Self-removal returns an error now, otherwise remaining members won't know about it. It should be disabled in UIs and the user should resolve it on their own (e.g. by asking other members). --- src/chat.rs | 9 +++++++-- src/receive_imf/receive_imf_tests.rs | 30 ++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 265710680..34d6d2729 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3945,7 +3945,7 @@ pub(crate) async fn add_contact_to_chat_ex( } add_to_chat_contacts_table(context, time(), chat_id, &[contact_id]).await?; } - if chat.typ == Chattype::Group && chat.is_promoted() { + if chat.typ == Chattype::Group && !chat.grpid.is_empty() && chat.is_promoted() { msg.viewtype = Viewtype::Text; let contact_addr = contact.get_addr().to_lowercase(); @@ -4118,6 +4118,11 @@ pub async fn remove_contact_from_chat( ); context.emit_event(EventType::ErrorSelfNotInGroup(err_msg.clone())); bail!("{}", err_msg); + } else if chat.typ == Chattype::Group + && chat.grpid.is_empty() + && contact_id == ContactId::SELF + { + bail!("Please ask other members to remove you from the thread"); } else { let mut sync = Nosync; @@ -4138,7 +4143,7 @@ pub async fn remove_contact_from_chat( // This allows to delete dangling references to deleted contacts // in case of the database becoming inconsistent due to a bug. if let Some(contact) = Contact::get_by_id_optional(context, contact_id).await? { - if chat.typ == Chattype::Group && chat.is_promoted() { + if chat.typ == Chattype::Group && !chat.grpid.is_empty() && chat.is_promoted() { msg.viewtype = Viewtype::Text; if contact_id == ContactId::SELF { msg.text = stock_str::msg_group_left_local(context, ContactId::SELF).await; diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index fd1838bf7..cc54d68fc 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -2808,11 +2808,15 @@ Second thread."#; let alice_fiona_contact_id = alice_fiona_contact.id; chat::add_contact_to_chat(&alice, alice_first_msg.chat_id, alice_fiona_contact_id).await?; - let alice_first_invite = alice.pop_sent_msg().await; + let alice_first_invite = alice + .send_text(alice_first_msg.chat_id, "I added Fiona") + .await; let fiona_first_invite = fiona.recv_msg(&alice_first_invite).await; chat::add_contact_to_chat(&alice, alice_second_msg.chat_id, alice_fiona_contact_id).await?; - let alice_second_invite = alice.pop_sent_msg().await; + let alice_second_invite = alice + .send_text(alice_second_msg.chat_id, "I added Fiona") + .await; let fiona_second_invite = fiona.recv_msg(&alice_second_invite).await; // Fiona was added to two separate chats and should see two separate chats, even though they @@ -4431,12 +4435,22 @@ async fn test_mua_can_readd() -> Result<()> { assert_eq!(alice_chat.typ, Chattype::Group); assert!(is_contact_in_chat(&alice, alice_chat.id, ContactId::SELF).await?); - // And leaves it. - remove_contact_from_chat(&alice, alice_chat.id, ContactId::SELF).await?; - alice.pop_sent_msg().await; - assert!(!is_contact_in_chat(&alice, alice_chat.id, ContactId::SELF).await?); + // Self-removal from unencrypted groups should fail. + assert!( + remove_contact_from_chat(&alice, alice_chat.id, ContactId::SELF) + .await + .is_err() + ); - // Bob uses a classical MUA to answer, adding Alice back. + // And removes Claire. + let claire_id = Contact::lookup_id_by_addr(&alice, "claire@example.org", Origin::OutgoingTo) + .await? + .unwrap(); + remove_contact_from_chat(&alice, alice_chat.id, claire_id).await?; + assert!(!alice.sql.exists("SELECT COUNT(*) FROM smtp", ()).await?); + assert!(!is_contact_in_chat(&alice, alice_chat.id, claire_id).await?); + + // Bob uses a classical MUA to answer, adding Claire back. receive_imf( &alice, b"Subject: Re: Message from alice\r\n\ @@ -4451,7 +4465,7 @@ async fn test_mua_can_readd() -> Result<()> { ) .await? .unwrap(); - assert!(is_contact_in_chat(&alice, alice_chat.id, ContactId::SELF).await?); + assert!(is_contact_in_chat(&alice, alice_chat.id, claire_id).await?); Ok(()) }