Skip to content

Commit

Permalink
feat: Delete vg-request-with-auth from IMAP after processing (#6208)
Browse files Browse the repository at this point in the history
In multi-device case `vg-request-with-auth` left on IMAP may result in situation when Bob joins the
group, then leaves it, then second Alice device comes online and processes `vg-request-with-auth`
again and adds Bob back. So we should IMAP-delete `vg-request-with-auth`. Another device will know
the Bob's key from Autocrypt-Gossip. It's not a problem if Alice loses state (restores from an old
backup) or goes offline for long before sending `vg-member-added`, anyway it may not be delivered by
the server, rather Bob should retry sending SecureJoin messages as he is a part which wants to join,
so let's not solve this for now.
  • Loading branch information
iequidoo committed Dec 25, 2024
1 parent 3d9aee1 commit 4e00521
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 6 deletions.
13 changes: 8 additions & 5 deletions deltachat-rpc-client/tests/test_securejoin.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,25 @@ def test_qr_securejoin(acfactory, protect, tmp_path):
qr_code = alice_chat.get_qr_code()
bob.secure_join(qr_code)

# Check that at least some of the handshake messages are deleted.
# Alice deletes "vg-request".
while True:
event = alice.wait_for_event()
if event["kind"] == "ImapMessageDeleted":
break
alice.wait_for_securejoin_inviter_success()
# Bob deletes "vg-auth-required", Alice deletes "vg-request-with-auth".
for ac in [alice, bob]:
while True:
event = ac.wait_for_event()
if event["kind"] == "ImapMessageDeleted":
break

alice.wait_for_securejoin_inviter_success()
bob.wait_for_securejoin_joiner_success()

# Test that Alice verified Bob's profile.
alice_contact_bob = alice.get_contact_by_addr(bob.get_config("addr"))
alice_contact_bob_snapshot = alice_contact_bob.get_snapshot()
assert alice_contact_bob_snapshot.is_verified

bob.wait_for_securejoin_joiner_success()

snapshot = bob.get_message_by_id(bob.wait_for_incoming_msg_event().msg_id).get_snapshot()
assert snapshot.text == "Member Me ({}) added by {}.".format(bob.get_config("addr"), alice.get_config("addr"))
assert snapshot.chat.get_basic_snapshot().is_protected == protect
Expand Down
1 change: 1 addition & 0 deletions src/headerdef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ pub enum HeaderDef {

/// [Autocrypt](https://autocrypt.org/) header.
Autocrypt,
AutocryptGossip,
AutocryptSetupMessage,
SecureJoin,

Expand Down
7 changes: 6 additions & 1 deletion src/securejoin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,9 @@ pub(crate) async fn handle_securejoin_handshake(
.await?;
inviter_progress(context, contact_id, 800);
inviter_progress(context, contact_id, 1000);
// IMAP-delete the message to avoid handling it by another device and adding the
// member twice. Another device will know the member's key from Autocrypt-Gossip.
Ok(HandshakeMessage::Done)
} else {
// Setup verified contact.
secure_connection_established(
Expand All @@ -468,8 +471,8 @@ pub(crate) async fn handle_securejoin_handshake(
.context("failed sending vc-contact-confirm message")?;

inviter_progress(context, contact_id, 1000);
Ok(HandshakeMessage::Ignore) // "Done" would delete the message and break multi-device (the key from Autocrypt-header is needed)
}
Ok(HandshakeMessage::Ignore) // "Done" would delete the message and break multi-device (the key from Autocrypt-header is needed)
}
/*=======================================================
==== Bob - the joiner's side ====
Expand Down Expand Up @@ -1353,6 +1356,8 @@ mod tests {
// explicit user action, the Auto-Submitted header shouldn't be present. Otherwise it would
// be strange to have it in "member-added" messages of verified groups only.
assert!(msg.get_header(HeaderDef::AutoSubmitted).is_none());
// This is a two-member group, but Alice must Autocrypt-gossip to her other devices.
assert!(msg.get_header(HeaderDef::AutocryptGossip).is_some());

{
// Now Alice's chat with Bob should still be hidden, the verified message should
Expand Down

0 comments on commit 4e00521

Please sign in to comment.