Skip to content

Commit

Permalink
fix: Save contact name from SecureJoin QR to authname, not to name (
Browse files Browse the repository at this point in the history
#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.
  • Loading branch information
iequidoo committed Nov 1, 2024
1 parent ded8c02 commit bea7e47
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 9 deletions.
5 changes: 4 additions & 1 deletion src/contact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions src/qr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result<Qr> {
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:?}"))?;

Expand Down Expand Up @@ -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(), "[email protected]");
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");
}
Expand All @@ -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(), "[email protected]");
assert_eq!(contact.get_authname(), "");
assert_eq!(contact.get_name(), "");
} else {
bail!("Wrong QR code type");
Expand Down
28 changes: 22 additions & 6 deletions src/securejoin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,7 @@ mod tests {
WrongAliceGossip,
SecurejoinWaitTimeout,
AliceIsBot,
AliceHasName,
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand Down

0 comments on commit bea7e47

Please sign in to comment.