diff --git a/src/mimefactory.rs b/src/mimefactory.rs index a81d3f1ed..a156622c9 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -142,6 +142,7 @@ struct MessageHeaders { impl<'a> MimeFactory<'a> { pub async fn from_msg(context: &Context, msg: &'a Message) -> Result> { let chat = Chat::load_from_db(context, msg.chat_id).await?; + let attach_profile_data = Self::should_attach_profile_data(msg); let from_addr = context.get_primary_self_addr().await?; let config_displayname = context @@ -152,7 +153,11 @@ impl<'a> MimeFactory<'a> { if let Some(override_name) = msg.param.get(Param::OverrideSenderDisplayname) { (override_name.to_string(), Some(config_displayname)) } else { - (config_displayname, None) + let name = match attach_profile_data { + true => config_displayname, + false => "".to_string(), + }; + (name, None) }; let mut recipients = Vec::with_capacity(5); @@ -186,7 +191,11 @@ impl<'a> MimeFactory<'a> { for row in rows { let (authname, addr, id) = row?; if !recipients_contain_addr(&recipients, &addr) { - recipients.push((authname, addr)); + let name = match attach_profile_data { + true => authname, + false => "".to_string(), + }; + recipients.push((name, addr)); } recipient_ids.insert(id); } @@ -220,16 +229,20 @@ impl<'a> MimeFactory<'a> { }, ) .await?; + let selfstatus = match attach_profile_data { + true => context + .get_config(Config::Selfstatus) + .await? + .unwrap_or_default(), + false => "".to_string(), + }; let attach_selfavatar = Self::should_attach_selfavatar(context, msg).await; let factory = MimeFactory { from_addr, from_displayname, sender_displayname, - selfstatus: context - .get_config(Config::Selfstatus) - .await? - .unwrap_or_default(), + selfstatus, recipients, timestamp: msg.timestamp_sort, loaded: Loaded::Message { chat }, @@ -396,31 +409,35 @@ impl<'a> MimeFactory<'a> { } } - async fn should_attach_selfavatar(context: &Context, msg: &Message) -> bool { - let cmd = msg.param.get_cmd(); - (cmd != SystemMessage::SecurejoinMessage || { + fn should_attach_profile_data(msg: &Message) -> bool { + msg.param.get_cmd() != SystemMessage::SecurejoinMessage || { let step = msg.param.get(Param::Arg).unwrap_or_default(); - // Don't attach selfavatar at the earlier SecureJoin steps: + // 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 - // deleted right after processing, so other devices won't see the avatar. + // deleted right after processing, so other devices won't see the avatar etc. // - 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 - } } } + async fn should_attach_selfavatar(context: &Context, msg: &Message) -> bool { + Self::should_attach_profile_data(msg) + && 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 } => { @@ -480,7 +497,11 @@ impl<'a> MimeFactory<'a> { return Ok(format!("Re: {}", remove_subject_prefix(last_subject))); } - let self_name = &match context.get_config(Config::Displayname).await? { + let self_name = match Self::should_attach_profile_data(self.msg) { + true => context.get_config(Config::Displayname).await?, + false => None, + }; + let self_name = &match self_name { Some(name) => name, None => context.get_config(Config::Addr).await?.unwrap_or_default(), }; @@ -708,7 +729,8 @@ impl<'a> MimeFactory<'a> { .unwrap(), ); } - if is_encrypted && verified { + if is_encrypted && verified || self.msg.param.get_cmd() == SystemMessage::SecurejoinMessage + { headers.unprotected.insert( 0, Header::new_with_value( diff --git a/src/securejoin.rs b/src/securejoin.rs index d643b2d68..a706dd5ff 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -802,6 +802,9 @@ mod tests { let alice = tcm.alice().await; let alice_addr = &alice.get_config(Config::Addr).await.unwrap().unwrap(); let bob = tcm.bob().await; + bob.set_config(Config::Displayname, Some("Bob Examplenet")) + .await + .unwrap(); alice .set_config(Config::VerifiedOneOnOneChats, Some("1")) .await @@ -824,6 +827,11 @@ mod tests { // Step 1: Generate QR-code, ChatId(0) indicates setup-contact let qr = get_securejoin_qr(&alice.ctx, None).await.unwrap(); + // We want Bob to learn Alice's name from their messages, not from the QR code. + alice + .set_config(Config::Displayname, Some("Alice Exampleorg")) + .await + .unwrap(); // Step 2: Bob scans QR-code, sends vc-request join_securejoin(&bob.ctx, &qr).await.unwrap(); @@ -895,6 +903,7 @@ mod tests { // Check Bob sent the right message. let sent = bob.pop_sent_msg().await; + assert!(!sent.payload.contains("Bob Examplenet")); let mut msg = alice.parse_msg(&sent).await; let vc_request_with_auth_ts_sent = msg .get_header(HeaderDef::Date) @@ -946,6 +955,7 @@ mod tests { .await .unwrap(); assert_eq!(contact_bob.is_verified(&alice.ctx).await.unwrap(), false); + assert_eq!(contact_bob.get_authname(), ""); if case == SetupContactCase::CheckProtectionTimestamp { SystemTime::shift(Duration::from_secs(3600)); @@ -954,6 +964,10 @@ mod tests { // Step 5+6: Alice receives vc-request-with-auth, sends vc-contact-confirm alice.recv_msg_trash(&sent).await; assert_eq!(contact_bob.is_verified(&alice.ctx).await.unwrap(), true); + let contact_bob = Contact::get_by_id(&alice.ctx, contact_bob_id) + .await + .unwrap(); + assert_eq!(contact_bob.get_authname(), "Bob Examplenet"); // exactly one one-to-one chat should be visible for both now // (check this before calling alice.create_chat() explicitly below) @@ -981,8 +995,19 @@ mod tests { } } + // Make sure Alice hasn't yet sent their name to Bob. + let contact_alice_id = Contact::lookup_id_by_addr(&bob.ctx, alice_addr, Origin::Unknown) + .await + .expect("Error looking up contact") + .expect("Contact not found"); + let contact_alice = Contact::get_by_id(&bob.ctx, contact_alice_id) + .await + .unwrap(); + assert_eq!(contact_alice.get_authname(), ""); + // Check Alice sent the right message to Bob. let sent = alice.pop_sent_msg().await; + assert!(!sent.payload.contains("Alice Exampleorg")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); assert_eq!( @@ -991,18 +1016,15 @@ mod tests { ); // Bob should not yet have Alice verified - let contact_alice_id = Contact::lookup_id_by_addr(&bob.ctx, alice_addr, Origin::Unknown) - .await - .expect("Error looking up contact") - .expect("Contact not found"); - let contact_alice = Contact::get_by_id(&bob.ctx, contact_alice_id) - .await - .unwrap(); - assert_eq!(contact_bob.is_verified(&bob.ctx).await.unwrap(), false); + assert_eq!(contact_alice.is_verified(&bob.ctx).await.unwrap(), false); // Step 7: Bob receives vc-contact-confirm bob.recv_msg_trash(&sent).await; assert_eq!(contact_alice.is_verified(&bob.ctx).await.unwrap(), true); + let contact_alice = Contact::get_by_id(&bob.ctx, contact_alice_id) + .await + .unwrap(); + assert_eq!(contact_alice.get_authname(), "Alice Exampleorg"); if case != SetupContactCase::SecurejoinWaitTimeout { // Later we check that the timeout message isn't added to the already protected chat. @@ -1119,7 +1141,7 @@ mod tests { // Alice should not yet have Bob verified let (contact_bob_id, _modified) = Contact::add_or_lookup( &alice.ctx, - "Bob", + "", &ContactAddress::new("bob@example.net")?, Origin::ManuallyCreated, ) diff --git a/src/test_utils.rs b/src/test_utils.rs index 4d0aee65a..d179785c8 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -566,18 +566,12 @@ impl TestContext { /// Returns the [`Contact`] for the other [`TestContext`], creating it if necessary. pub async fn add_or_lookup_contact(&self, other: &TestContext) -> Contact { - let name = other - .ctx - .get_config(Config::Displayname) - .await - .unwrap_or_default() - .unwrap_or_default(); let primary_self_addr = other.ctx.get_primary_self_addr().await.unwrap(); let addr = ContactAddress::new(&primary_self_addr).unwrap(); // MailinglistAddress is the lowest allowed origin, we'd prefer to not modify the // origin when creating this contact. let (contact_id, modified) = - Contact::add_or_lookup(self, &name, &addr, Origin::MailinglistAddress) + Contact::add_or_lookup(self, "", &addr, Origin::MailinglistAddress) .await .expect("add_or_lookup"); match modified {