Skip to content

Commit f61d5af

Browse files
committed
feat: Delete vg-request-with-auth from IMAP after processing (#6208)
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.
1 parent 3d9aee1 commit f61d5af

File tree

3 files changed

+15
-6
lines changed

3 files changed

+15
-6
lines changed

deltachat-rpc-client/tests/test_securejoin.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,22 +73,25 @@ def test_qr_securejoin(acfactory, protect, tmp_path):
7373
qr_code = alice_chat.get_qr_code()
7474
bob.secure_join(qr_code)
7575

76-
# Check that at least some of the handshake messages are deleted.
76+
# Alice deletes "vg-request".
77+
while True:
78+
event = alice.wait_for_event()
79+
if event["kind"] == "ImapMessageDeleted":
80+
break
81+
alice.wait_for_securejoin_inviter_success()
82+
# Bob deletes "vg-auth-required", Alice deletes "vg-request-with-auth".
7783
for ac in [alice, bob]:
7884
while True:
7985
event = ac.wait_for_event()
8086
if event["kind"] == "ImapMessageDeleted":
8187
break
82-
83-
alice.wait_for_securejoin_inviter_success()
88+
bob.wait_for_securejoin_joiner_success()
8489

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

90-
bob.wait_for_securejoin_joiner_success()
91-
9295
snapshot = bob.get_message_by_id(bob.wait_for_incoming_msg_event().msg_id).get_snapshot()
9396
assert snapshot.text == "Member Me ({}) added by {}.".format(bob.get_config("addr"), alice.get_config("addr"))
9497
assert snapshot.chat.get_basic_snapshot().is_protected == protect

src/headerdef.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ pub enum HeaderDef {
7373

7474
/// [Autocrypt](https://autocrypt.org/) header.
7575
Autocrypt,
76+
AutocryptGossip,
7677
AutocryptSetupMessage,
7778
SecureJoin,
7879

src/securejoin.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,9 @@ pub(crate) async fn handle_securejoin_handshake(
454454
.await?;
455455
inviter_progress(context, contact_id, 800);
456456
inviter_progress(context, contact_id, 1000);
457+
// IMAP-delete the message to avoid handling it by another device and adding the
458+
// member twice. Another device will know the member's key from Autocrypt-Gossip.
459+
Ok(HandshakeMessage::Done)
457460
} else {
458461
// Setup verified contact.
459462
secure_connection_established(
@@ -468,8 +471,8 @@ pub(crate) async fn handle_securejoin_handshake(
468471
.context("failed sending vc-contact-confirm message")?;
469472

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

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

0 commit comments

Comments
 (0)