diff --git a/python/tests/test_0_complex_or_slow.py b/python/tests/test_0_complex_or_slow.py index 11f8dd6c3..4ee773912 100644 --- a/python/tests/test_0_complex_or_slow.py +++ b/python/tests/test_0_complex_or_slow.py @@ -547,13 +547,14 @@ def test_see_new_verified_member_after_going_online(acfactory, tmp_path, lp): assert msg_in.get_sender_contact().addr == ac2_addr -def test_use_new_verified_group_after_going_online(acfactory, tmp_path, lp): +def test_use_new_verified_group_after_going_online(acfactory, data, tmp_path, lp): """Another test for the bug #3836: - Bob has two devices, the second is offline. - Alice creates a verified group and sends a QR invitation to Bob. - Bob joins the group. - Bob's second devices goes online, but sees a contact request instead of the verified group. - The "member added" message is not a system message but a plain text message. + - Bob's second device doesn't display the Alice's avatar (bug #5354). - Sending a message fails as the key is missing -- message info says "proper enc-key for missing, cannot encrypt". """ @@ -568,6 +569,10 @@ def test_use_new_verified_group_after_going_online(acfactory, tmp_path, lp): ac2_offl.import_self_keys(str(dir)) ac2_offl.stop_io() + lp.sec("ac1: set avatar") + avatar_path = data.get_path("d.png") + ac1.set_avatar(avatar_path) + lp.sec("ac1: create verified-group QR, ac2 scans and joins") chat = ac1.create_group_chat("hello", verified=True) assert chat.is_protected() @@ -580,11 +585,13 @@ def test_use_new_verified_group_after_going_online(acfactory, tmp_path, lp): ac2_offl.start_io() # Receive "Member Me () added by ." message. msg_in = ac2_offl._evtracker.wait_next_incoming_message() + contact = msg_in.get_sender_contact() assert msg_in.is_system_message() - assert msg_in.get_sender_contact().addr == ac1.get_config("addr") + assert contact.addr == ac1.get_config("addr") chat2 = msg_in.chat assert chat2.is_protected() assert chat2.get_messages()[0].text == "Messages are guaranteed to be end-to-end encrypted from now on." + assert open(contact.get_profile_image(), "rb").read() == open(avatar_path, "rb").read() lp.sec("ac2_offl: sending message") msg_out = chat2.send_text("hello") diff --git a/src/chat.rs b/src/chat.rs index 1af3feb28..e7b75e4bf 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2742,17 +2742,8 @@ async fn prepare_send_msg( /// The caller has to interrupt SMTP loop or otherwise process new rows. pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -> Result> { let needs_encryption = msg.param.get_bool(Param::GuaranteeE2ee).unwrap_or_default(); - - let attach_selfavatar = match shall_attach_selfavatar(context, msg.chat_id).await { - Ok(attach_selfavatar) => attach_selfavatar, - Err(err) => { - warn!(context, "SMTP job cannot get selfavatar-state: {err:#}."); - false - } - }; - - let mimefactory = MimeFactory::from_msg(context, msg, attach_selfavatar).await?; - + let mimefactory = MimeFactory::from_msg(context, msg).await?; + let attach_selfavatar = mimefactory.attach_selfavatar; let mut recipients = mimefactory.recipients(); let from = context.get_primary_self_addr().await?; diff --git a/src/mimefactory.rs b/src/mimefactory.rs index ddb69fadd..46fe794c3 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -10,7 +10,7 @@ use lettre_email::{Address, Header, MimeMultipartType, PartBuilder}; use tokio::fs; use crate::blob::BlobObject; -use crate::chat::Chat; +use crate::chat::{self, Chat}; use crate::config::Config; use crate::constants::{Chattype, DC_FROM_HANDSHAKE}; use crate::contact::Contact; @@ -83,7 +83,7 @@ pub struct MimeFactory<'a> { sync_ids_to_delete: Option, /// True if the avatar should be attached. - attach_selfavatar: bool, + pub attach_selfavatar: bool, } /// Result of rendering a message, ready to be submitted to a send job. @@ -138,11 +138,7 @@ struct MessageHeaders { } impl<'a> MimeFactory<'a> { - pub async fn from_msg( - context: &Context, - msg: &'a Message, - attach_selfavatar: bool, - ) -> Result> { + pub async fn from_msg(context: &Context, msg: &'a Message) -> Result> { let chat = Chat::load_from_db(context, msg.chat_id).await?; let from_addr = context.get_primary_self_addr().await?; @@ -217,6 +213,7 @@ impl<'a> MimeFactory<'a> { }, ) .await?; + let attach_selfavatar = Self::should_attach_selfavatar(context, msg).await; let factory = MimeFactory { from_addr, @@ -392,6 +389,31 @@ impl<'a> MimeFactory<'a> { } } + async fn should_attach_selfavatar(context: &Context, msg: &Message) -> bool { + let cmd = msg.param.get_cmd(); + (cmd != SystemMessage::SecurejoinMessage || { + let step = msg.param.get(Param::Arg).unwrap_or_default(); + // Don't attach selfavatar at the earlier SecureJoin steps: + // - The corresponding messages, i.e. "v{c,g}-request" and "v{c,g}-auth-required" are + // deleted right after processing, so other devices won't see the avatar. + // - It's also good for privacy because the contact isn't yet verified and these + // messages are auto-sent unlike usual unencrypted messages. + step == "vg-request-with-auth" + || step == "vc-request-with-auth" + || step == "vg-member-added" + || step == "vc-contact-confirm" + }) && match chat::shall_attach_selfavatar(context, msg.chat_id).await { + Ok(should) => should, + Err(err) => { + warn!( + context, + "should_attach_selfavatar: cannot get selfavatar state: {err:#}." + ); + false + } + } + } + fn grpimage(&self) -> Option { match &self.loaded { Loaded::Message { chat } => { @@ -1836,7 +1858,7 @@ mod tests { Original-Message-ID: <2893@example.com>\n\ Disposition: manual-action/MDN-sent-automatically; displayed\n\ \n", &t).await; - let mf = MimeFactory::from_msg(&t, &new_msg, false).await.unwrap(); + let mf = MimeFactory::from_msg(&t, &new_msg).await.unwrap(); // The subject string should not be "Re: message opened" assert_eq!("Re: Hello, Bob", mf.subject_str(&t).await.unwrap()); } @@ -1971,7 +1993,7 @@ mod tests { new_msg.chat_id = chat_id; chat::prepare_msg(&t, chat_id, &mut new_msg).await.unwrap(); - let mf = MimeFactory::from_msg(&t, &new_msg, false).await.unwrap(); + let mf = MimeFactory::from_msg(&t, &new_msg).await.unwrap(); mf.subject_str(&t).await.unwrap() } @@ -2056,7 +2078,7 @@ mod tests { new_msg.set_quote(&t, Some(&incoming_msg)).await.unwrap(); } - let mf = MimeFactory::from_msg(&t, &new_msg, false).await.unwrap(); + let mf = MimeFactory::from_msg(&t, &new_msg).await.unwrap(); mf.subject_str(&t).await.unwrap() } @@ -2104,7 +2126,7 @@ mod tests { ) .await; - let mimefactory = MimeFactory::from_msg(&t, &msg, false).await.unwrap(); + let mimefactory = MimeFactory::from_msg(&t, &msg).await.unwrap(); let recipients = mimefactory.recipients(); assert_eq!(recipients, vec!["charlie@example.com"]);