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. But also we should make sure that `vg-member-added` is sent
before that. For this, add a new `imap.target_min_smtp_id` column and only move or delete emails
when `smtp.id` reaches the `imap.target_min_smtp_id` value.

src/headerdef.rs

@@ -73,6 +73,7 @@ pub enum HeaderDef {
 
     /// [Autocrypt](https://autocrypt.org/) header.
     Autocrypt,
+    AutocryptGossip,
     AutocryptSetupMessage,
     SecureJoin,
 

src/imap.rs

@@ -1017,14 +1017,20 @@ impl Session {
     ///
     /// This is the only place where messages are moved or deleted on the IMAP server.
     async fn move_delete_messages(&mut self, context: &Context, folder: &str) -> Result<()> {
+        let target_min_smtp_id: i64 = context
+            .sql
+            .query_get_value("SELECT IFNULL((SELECT MIN(id) FROM smtp), 0)", ())
+            .await?
+            .context("No value??")?;
         let rows = context
             .sql
             .query_map(
                 "SELECT id, uid, target FROM imap
-        WHERE folder = ?
-        AND target != folder
-        ORDER BY target, uid",
-                (folder,),
+                WHERE folder = ?
+                    AND target != folder
+                    AND target_min_smtp_id <= ?
+                ORDER BY target, uid",
+                (folder, target_min_smtp_id),
                 |row| {
                     let rowid: i64 = row.get(0)?;
                     let uid: u32 = row.get(1)?;

src/receive_imf.rs

@@ -60,7 +60,8 @@ pub struct ReceivedMsg {
     /// IDs of inserted rows in messages table.
     pub msg_ids: Vec<MsgId>,
 
-    /// Whether IMAP messages should be immediately deleted.
+    /// Whether IMAP messages should be deleted. Deletion is done after necessary auto-generated
+    /// messages are sent which is important for SecureJoin.
     pub needs_delete_job: bool,
 
     /// Whether the From address was repeated in the signed part
@@ -587,12 +588,27 @@ pub(crate) async fn receive_imf_inner(
                 Some(_) => "target=?1,",
                 None => "",
             };
+            let target_min_smtp_id: i64 = match received_msg.needs_delete_job {
+                true => context
+                    .sql
+                    .query_get_value("SELECT IFNULL((SELECT MAX(id) + 1 FROM smtp), 0)", ())
+                    .await?
+                    .context("No value??")?,
+                false => 0,
+            };
             context
                 .sql
                 .execute(
-                    &format!("UPDATE imap SET {target_subst} rfc724_mid=?2 WHERE rfc724_mid=?3"),
+                    &format!(
+                        "UPDATE imap SET
+                            {target_subst}
+                            target_min_smtp_id=?2,
+                            rfc724_mid=?3
+                        WHERE rfc724_mid=?4"
+                    ),
                     (
                         target.as_deref().unwrap_or_default(),
+                        target_min_smtp_id,
                         rfc724_mid_orig,
                         rfc724_mid,
                     ),

src/securejoin.rs

@@ -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(
@@ -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             ====
@@ -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

src/sql/migrations.rs

@@ -1121,6 +1121,18 @@ CREATE INDEX msgs_status_updates_index2 ON msgs_status_updates (uid);
         .await?;
     }
 
+    inc_and_check(&mut migration_version, 127)?;
+    if dbversion < migration_version {
+        // Emails must be moved to `target` or deleted only when `smtp.id` reaches
+        // `target_min_smtp_id` to keep SecureJoin working for multi-device and in case of a local
+        // state loss.
+        sql.execute_migration(
+            "ALTER TABLE imap ADD COLUMN target_min_smtp_id INTEGER NOT NULL DEFAULT 0",
+            migration_version,
+        )
+        .await?;
+    }
+
     let new_version = sql
         .get_raw_config_int(VERSION_CFG)
         .await?