From bea7e4792c299fdb6973c16d3e6d4b3b534927cd Mon Sep 17 00:00:00 2001 From: iequidoo Date: Thu, 31 Oct 2024 18:01:58 -0300 Subject: [PATCH] fix: Save contact name from SecureJoin QR to `authname`, not to `name` (#6115) 3f9242a saves name from all QR codes to `name` (i.e. manually edited name), but for SecureJoin QR codes the name should be saved to `authname` because such QR codes are generated by the inviter. Other QR codes may be generated locally and not only by Delta Chat, so the name from them mustn't go to `authname` and be revealed to the network or other contacts. --- src/contact.rs | 5 ++++- src/qr.rs | 6 ++++-- src/securejoin.rs | 28 ++++++++++++++++++++++------ 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/contact.rs b/src/contact.rs index b6096ed9b6..4d8704f246 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -425,9 +425,12 @@ pub enum Origin { /// To: of incoming messages of unknown sender IncomingUnknownTo = 0x40, - /// address scanned but not verified + /// Address scanned but not verified. UnhandledQrScan = 0x80, + /// Address scanned from a SecureJoin QR code, but not verified yet. + UnhandledSecurejoinQrScan = 0x81, + /// Reply-To: of incoming message of known sender /// Contacts with at least this origin value are shown in the contact list. IncomingReplyTo = 0x100, diff --git a/src/qr.rs b/src/qr.rs index 22cfa2b958..dee84d5329 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -446,7 +446,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { if let (Some(addr), Some(invitenumber), Some(authcode)) = (&addr, invitenumber, authcode) { let addr = ContactAddress::new(addr)?; let (contact_id, _) = - Contact::add_or_lookup(context, &name, &addr, Origin::UnhandledQrScan) + Contact::add_or_lookup(context, &name, &addr, Origin::UnhandledSecurejoinQrScan) .await .with_context(|| format!("failed to add or lookup contact for address {addr:?}"))?; @@ -1270,7 +1270,8 @@ mod tests { if let Qr::AskVerifyContact { contact_id, .. } = qr { let contact = Contact::get_by_id(&ctx.ctx, contact_id).await?; assert_eq!(contact.get_addr(), "cli@deltachat.de"); - assert_eq!(contact.get_name(), "Jörn P. P."); + assert_eq!(contact.get_authname(), "Jörn P. P."); + assert_eq!(contact.get_name(), ""); } else { bail!("Wrong QR code type"); } @@ -1285,6 +1286,7 @@ mod tests { if let Qr::AskVerifyContact { contact_id, .. } = qr { let contact = Contact::get_by_id(&ctx.ctx, contact_id).await?; assert_eq!(contact.get_addr(), "cli@deltachat.de"); + assert_eq!(contact.get_authname(), ""); assert_eq!(contact.get_name(), ""); } else { bail!("Wrong QR code type"); diff --git a/src/securejoin.rs b/src/securejoin.rs index 3b2a7ecb4d..9106e0cdd7 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -765,6 +765,7 @@ mod tests { WrongAliceGossip, SecurejoinWaitTimeout, AliceIsBot, + AliceHasName, } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -792,10 +793,21 @@ mod tests { test_setup_contact_ex(SetupContactCase::AliceIsBot).await } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_setup_contact_alice_has_name() { + test_setup_contact_ex(SetupContactCase::AliceHasName).await + } + async fn test_setup_contact_ex(case: SetupContactCase) { let mut tcm = TestContextManager::new(); let alice = tcm.alice().await; let alice_addr = &alice.get_config(Config::Addr).await.unwrap().unwrap(); + if case == SetupContactCase::AliceHasName { + alice + .set_config(Config::Displayname, Some("Alice")) + .await + .unwrap(); + } let bob = tcm.bob().await; bob.set_config(Config::Displayname, Some("Bob Examplenet")) .await @@ -840,7 +852,10 @@ mod tests { Chatlist::try_load(&bob, 0, None, None).await.unwrap().len(), 1 ); - + 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 sent = bob.pop_sent_msg().await; assert!(!sent.payload.contains("Bob Examplenet")); assert_eq!(sent.recipient(), EmailAddress::new(alice_addr).unwrap()); @@ -974,6 +989,7 @@ mod tests { .await .unwrap(); assert_eq!(contact_bob.get_authname(), "Bob Examplenet"); + assert!(contact_bob.get_name().is_empty()); assert_eq!(contact_bob.is_bot(), false); // exactly one one-to-one chat should be visible for both now @@ -1003,14 +1019,13 @@ 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(), ""); + match case { + SetupContactCase::AliceHasName => assert_eq!(contact_alice.get_authname(), "Alice"), + _ => assert_eq!(contact_alice.get_authname(), ""), + }; // Check Alice sent the right message to Bob. let sent = alice.pop_sent_msg().await; @@ -1033,6 +1048,7 @@ mod tests { .await .unwrap(); assert_eq!(contact_alice.get_authname(), "Alice Exampleorg"); + assert!(contact_alice.get_name().is_empty()); assert_eq!(contact_alice.is_bot(), case == SetupContactCase::AliceIsBot); if case != SetupContactCase::SecurejoinWaitTimeout {