From 6fd088a4cc806a087b1eb26f29a9f3aaa7b9c32b Mon Sep 17 00:00:00 2001 From: iequidoo Date: Sat, 19 Apr 2025 16:24:25 -0300 Subject: [PATCH] feat: Don't attach profile data in group leave messages I think we can't use group leave messages as a really useful transport of profile data, i.e. if the user rarely send messages, sending their profile data in group leave messages doesn't solve the problem of stale profile data completely, but that may create privacy issues, e.g. the user may want to leave a group because it's not private enough and the user doesn't want to share their data with some members whom they may not even know, the user might be added by mistake etc. In the end, if the user really wants to share their data, they can send a farewell message. --- src/chat/chat_tests.rs | 34 ++++++++++++++++++++++++++++++++++ src/mimefactory.rs | 24 +++++++++++++++--------- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index e9980f97e..cdf026eb8 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -1589,6 +1589,40 @@ async fn test_profile_data_on_group_leave() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_no_profile_data_on_group_leave() -> Result<()> { + let mut tcm = TestContextManager::new(); + let t = &tcm.alice().await; + let chat_id = create_group_chat(t, ProtectionStatus::Unprotected, "foo").await?; + + let (contact_id, _) = Contact::add_or_lookup( + t, + "", + &ContactAddress::new("foo@bar.org")?, + Origin::IncomingUnknownTo, + ) + .await?; + add_contact_to_chat(t, chat_id, contact_id).await?; + + send_text_msg(t, chat_id, "populate".to_string()).await?; + t.pop_sent_msg().await; + + t.set_config(Config::Displayname, Some("aae7b258ad3cabd9")) + .await?; + let file = t.dir.path().join("avatar.png"); + let bytes = include_bytes!("../../test-data/image/avatar64x64.png"); + tokio::fs::write(&file, bytes).await?; + t.set_config(Config::Selfavatar, Some(file.to_str().unwrap())) + .await?; + assert!(shall_attach_selfavatar(t, chat_id).await?); + + remove_contact_from_chat(t, chat_id, ContactId::SELF).await?; + let sent_msg = t.pop_sent_msg().await; + assert!(!sent_msg.payload().contains("aae7b258ad3cabd9")); + assert!(!sent_msg.payload().contains("Chat-User-Avatar")); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_set_mute_duration() { let t = TestContext::new().await; diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 0d971bf24..2fe6aba04 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -181,7 +181,7 @@ impl MimeFactory { pub async fn from_msg(context: &Context, msg: Message) -> Result { let now = time(); let chat = Chat::load_from_db(context, msg.chat_id).await?; - let attach_profile_data = Self::should_attach_profile_data(&msg); + let attach_profile_data = Self::should_attach_profile_data(context, &msg).await?; let undisclosed_recipients = chat.typ == Chattype::OutBroadcast; let from_addr = context.get_primary_self_addr().await?; @@ -460,7 +460,7 @@ impl MimeFactory { .unwrap_or_default(), false => "".to_string(), }; - let attach_selfavatar = Self::should_attach_selfavatar(context, &msg).await; + let attach_selfavatar = Self::should_attach_selfavatar(context, &msg).await?; ensure_and_debug_assert!( member_timestamps.is_empty() @@ -553,8 +553,14 @@ impl MimeFactory { } } - fn should_attach_profile_data(msg: &Message) -> bool { - msg.param.get_cmd() != SystemMessage::SecurejoinMessage || { + async fn should_attach_profile_data(context: &Context, msg: &Message) -> Result { + if msg.param.get_cmd() == SystemMessage::MemberRemovedFromGroup + && msg.param.get(Param::Arg) + == context.get_config(Config::ConfiguredAddr).await?.as_deref() + { + return Ok(false); + } + Ok(msg.param.get_cmd() != SystemMessage::SecurejoinMessage || { let step = msg.param.get(Param::Arg).unwrap_or_default(); // Don't attach profile data at the earlier SecureJoin steps: // - The corresponding messages, i.e. "v{c,g}-request" and "v{c,g}-auth-required" are @@ -565,11 +571,11 @@ impl MimeFactory { || step == "vc-request-with-auth" || step == "vg-member-added" || step == "vc-contact-confirm" - } + }) } - async fn should_attach_selfavatar(context: &Context, msg: &Message) -> bool { - Self::should_attach_profile_data(msg) + async fn should_attach_selfavatar(context: &Context, msg: &Message) -> Result { + Ok(Self::should_attach_profile_data(context, msg).await? && match chat::shall_attach_selfavatar(context, msg.chat_id).await { Ok(should) => should, Err(err) => { @@ -579,7 +585,7 @@ impl MimeFactory { ); false } - } + }) } fn grpimage(&self) -> Option { @@ -640,7 +646,7 @@ impl MimeFactory { return Ok(format!("Re: {}", remove_subject_prefix(last_subject))); } - let self_name = match Self::should_attach_profile_data(msg) { + let self_name = match Self::should_attach_profile_data(context, msg).await? { true => context.get_config(Config::Displayname).await?, false => None, };