From b2aeb0cfe9c9c2682ea690357b3552e77aae6feb 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 0bb4ecd209..51f8c6e131 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -1503,6 +1503,40 @@ async fn test_shall_attach_selfavatar() -> 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 af3596c052..ec0eab26f5 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -161,7 +161,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::Broadcast; let from_addr = context.get_primary_self_addr().await?; @@ -312,7 +312,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?; debug_assert!( member_timestamps.is_empty() @@ -434,8 +434,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 @@ -446,11 +452,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) => { @@ -460,7 +466,7 @@ impl MimeFactory { ); false } - } + }) } fn grpimage(&self) -> Option { @@ -521,7 +527,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, };