From 418dfbf9946027567d598fa94e528c0ec44e34be Mon Sep 17 00:00:00 2001
From: iequidoo <dgreshilov@gmail.com>
Date: Wed, 4 Sep 2024 13:26:56 -0300
Subject: [PATCH] fix: Don't sync QR code token before populating the group
 (#5935)

Otherwise other devices don't yet know about the group and can't handle the sync message correctly.
---
 deltachat-rpc-client/tests/test_securejoin.py |  9 ++++-
 src/chat.rs                                   | 25 +++++++------
 src/sync.rs                                   | 35 +++++++++++++++++++
 3 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/deltachat-rpc-client/tests/test_securejoin.py b/deltachat-rpc-client/tests/test_securejoin.py
index 8969de01dc..7a0fdbaae3 100644
--- a/deltachat-rpc-client/tests/test_securejoin.py
+++ b/deltachat-rpc-client/tests/test_securejoin.py
@@ -62,7 +62,7 @@ def test_qr_setup_contact_svg(acfactory) -> None:
 
 @pytest.mark.parametrize("protect", [True, False])
 def test_qr_securejoin(acfactory, protect, tmp_path):
-    alice, bob = acfactory.get_online_accounts(2)
+    alice, bob, fiona = acfactory.get_online_accounts(3)
 
     # Setup second device for Alice
     # to test observing securejoin protocol.
@@ -112,6 +112,13 @@ def test_qr_securejoin(acfactory, protect, tmp_path):
     alice2_contact_bob_snapshot = alice2_contact_bob.get_snapshot()
     assert alice2_contact_bob_snapshot.is_verified
 
+    # The QR code token is synced, so alice2 must be able to handle join requests.
+    logging.info("Fiona joins verified group via alice2")
+    alice.stop_io()
+    fiona.secure_join(qr_code)
+    alice2.wait_for_securejoin_inviter_success()
+    fiona.wait_for_securejoin_joiner_success()
+
 
 def test_qr_securejoin_contact_request(acfactory) -> None:
     """Alice invites Bob to a group when Bob's chat with Alice is in a contact request mode."""
diff --git a/src/chat.rs b/src/chat.rs
index 92f4902fb1..a146a74321 100644
--- a/src/chat.rs
+++ b/src/chat.rs
@@ -1934,8 +1934,8 @@ impl Chat {
             msg.param.set_int(Param::AttachGroupImage, 1);
             self.param.remove(Param::Unpromoted);
             self.update_param(context).await?;
-            // send_sync_msg() is called (usually) a moment later at send_msg_to_smtp()
-            // when the group creation message is actually sent through SMTP --
+            // send_sync_msg() is called a moment later at `smtp::send_smtp_messages()`
+            // when the group creation message is already in the `smtp` table --
             // this makes sure, the other devices are aware of grpid that is used in the sync-message.
             context
                 .sync_qr_code_tokens(Some(self.id))
@@ -3726,17 +3726,13 @@ pub(crate) async fn add_contact_to_chat_ex(
         bail!("can not add contact because the account is not part of the group/broadcast");
     }
 
+    let sync_qr_code_tokens;
     if from_handshake && chat.param.get_int(Param::Unpromoted).unwrap_or_default() == 1 {
         chat.param.remove(Param::Unpromoted);
         chat.update_param(context).await?;
-        if context
-            .sync_qr_code_tokens(Some(chat_id))
-            .await
-            .log_err(context)
-            .is_ok()
-        {
-            context.scheduler.interrupt_smtp().await;
-        }
+        sync_qr_code_tokens = true;
+    } else {
+        sync_qr_code_tokens = false;
     }
 
     if context.is_self_addr(contact.get_addr()).await? {
@@ -3780,6 +3776,15 @@ pub(crate) async fn add_contact_to_chat_ex(
             return Err(e);
         }
         sync = Nosync;
+        if sync_qr_code_tokens
+            && context
+                .sync_qr_code_tokens(Some(chat_id))
+                .await
+                .log_err(context)
+                .is_ok()
+        {
+            context.scheduler.interrupt_smtp().await;
+        }
     }
     context.emit_event(EventType::ChatModified(chat_id));
     if sync.into() {
diff --git a/src/sync.rs b/src/sync.rs
index 29ad69c148..a60ec40a0c 100644
--- a/src/sync.rs
+++ b/src/sync.rs
@@ -328,8 +328,10 @@ mod tests {
     use anyhow::bail;
 
     use super::*;
+    use crate::chat::ProtectionStatus;
     use crate::chatlist::Chatlist;
     use crate::contact::{Contact, Origin};
+    use crate::securejoin::get_securejoin_qr;
     use crate::test_utils::{TestContext, TestContextManager};
     use crate::tools::SystemTime;
 
@@ -630,4 +632,37 @@ mod tests {
         assert_eq!(msg.text, "hi");
         Ok(())
     }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_unpromoted_group_no_qr_sync() -> Result<()> {
+        let mut tcm = TestContextManager::new();
+        let alice = &tcm.alice().await;
+        alice.set_config_bool(Config::SyncMsgs, true).await?;
+        let alice_chatid =
+            chat::create_group_chat(alice, ProtectionStatus::Protected, "the chat").await?;
+        let qr = get_securejoin_qr(alice, Some(alice_chatid)).await?;
+        let msg_id = alice.send_sync_msg().await?;
+        assert!(msg_id.is_none());
+
+        let bob = &tcm.bob().await;
+        tcm.exec_securejoin_qr(bob, alice, &qr).await;
+        let msg_id = alice.send_sync_msg().await?;
+        // The group becomes promoted when Bob joins, so the QR code token is synced.
+        assert!(msg_id.is_some());
+        let sent = alice.pop_sent_msg().await;
+        let msg = alice.parse_msg(&sent).await;
+        let mut sync_items = msg.sync_items.unwrap().items;
+        assert_eq!(sync_items.len(), 1);
+        let data = sync_items.pop().unwrap().data;
+        let SyncDataOrUnknown::SyncData(AddQrToken(_)) = data else {
+            unreachable!();
+        };
+
+        let fiona = &tcm.fiona().await;
+        tcm.exec_securejoin_qr(fiona, alice, &qr).await;
+        let msg_id = alice.send_sync_msg().await?;
+        // The QR code token was already synced before.
+        assert!(msg_id.is_none());
+        Ok(())
+    }
 }