fix: Let securejoin succeed even if the chat was deleted in the meantime (#7594)

Fix #7478 by creating the 1:1
chat in `handle_auth_required` if it doesn't exist anymore.

Author: Hocuri

src/securejoin/bob.rs

@@ -43,17 +43,7 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul
     // A 1:1 chat is needed to send messages to Alice.  When joining a group this chat is
     // hidden, if a user starts sending messages in it it will be unhidden in
     // receive_imf.
-    let hidden = match invite {
-        QrInvite::Contact { .. } => Blocked::Not,
-        QrInvite::Group { .. } => Blocked::Yes,
-        QrInvite::Broadcast { .. } => Blocked::Yes,
-    };
-
-    // The 1:1 chat with the inviter
-    let private_chat_id =
-        ChatId::create_for_contact_with_blocked(context, invite.contact_id(), hidden)
-            .await
-            .with_context(|| format!("can't create chat for contact {}", invite.contact_id()))?;
+    let private_chat_id = private_chat_id(context, &invite).await?;
 
     ContactId::scaleup_origin(context, &[invite.contact_id()], Origin::SecurejoinJoined).await?;
     context.emit_event(EventType::ContactsChanged(None));
@@ -175,6 +165,9 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul
 ///
 /// Returns the ID of the newly inserted entry.
 async fn insert_new_db_entry(context: &Context, invite: QrInvite, chat_id: ChatId) -> Result<i64> {
+    // The `chat_id` isn't actually needed anymore,
+    // but we still save it;
+    // can be removed as a future improvement.
     context
         .sql
         .insert(
@@ -195,11 +188,10 @@ pub(super) async fn handle_auth_required(
     // Load all Bob states that expect `vc-auth-required` or `vg-auth-required`.
     let bob_states = context
         .sql
-        .query_map_vec("SELECT id, invite, chat_id FROM bobstate", (), |row| {
+        .query_map_vec("SELECT id, invite FROM bobstate", (), |row| {
             let row_id: i64 = row.get(0)?;
             let invite: QrInvite = row.get(1)?;
-            let chat_id: ChatId = row.get(2)?;
-            Ok((row_id, invite, chat_id))
+            Ok((row_id, invite))
         })
         .await?;
 
@@ -209,7 +201,7 @@ pub(super) async fn handle_auth_required(
     );
 
     let mut auth_sent = false;
-    for (bobstate_row_id, invite, chat_id) in bob_states {
+    for (bobstate_row_id, invite) in bob_states {
         if !encrypted_and_signed(context, message, invite.fingerprint()) {
             continue;
         }
@@ -220,6 +212,7 @@ pub(super) async fn handle_auth_required(
         }
 
         info!(context, "Fingerprint verified.",);
+        let chat_id = private_chat_id(context, &invite).await?;
         send_handshake_message(context, &invite, chat_id, BobHandshakeMsg::RequestWithAuth).await?;
         context
             .sql
@@ -348,6 +341,22 @@ impl BobHandshakeMsg {
     }
 }
 
+/// Returns the 1:1 chat with the inviter.
+///
+/// This is the chat in which securejoin messages are sent.
+/// The 1:1 chat will be created if it does not yet exist.
+async fn private_chat_id(context: &Context, invite: &QrInvite) -> Result<ChatId> {
+    let hidden = match invite {
+        QrInvite::Contact { .. } => Blocked::Not,
+        QrInvite::Group { .. } => Blocked::Yes,
+        QrInvite::Broadcast { .. } => Blocked::Yes,
+    };
+
+    ChatId::create_for_contact_with_blocked(context, invite.contact_id(), hidden)
+        .await
+        .with_context(|| format!("can't create chat for contact {}", invite.contact_id()))
+}
+
 /// Returns the [`ChatId`] of the chat being joined.
 ///
 /// This is the chat in which you want to notify the user as well.

src/securejoin/securejoin_tests.rs

@@ -243,7 +243,7 @@ async fn test_setup_contact_ex(case: SetupContactCase) {
         .unwrap();
     match case {
         SetupContactCase::AliceHasName => assert_eq!(contact_alice.get_authname(), "Alice"),
-        _ => assert_eq!(contact_alice.get_authname(), "Alice Exampleorg"),
+        _ => assert_eq!(contact_alice.get_authname(), ""),
     };
 
     // Check Alice sent the right message to Bob.
@@ -1217,3 +1217,33 @@ async fn test_qr_no_implicit_inviter_addition() -> Result<()> {
 
     Ok(())
 }
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_user_deletes_chat_before_securejoin_completes() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+
+    let qr = get_securejoin_qr(alice, None).await?;
+    let bob_chat_id = join_securejoin(bob, &qr).await?;
+
+    let bob_alice_chat = bob.get_chat(alice).await;
+    // It's not possible yet to send to the chat, because Bob doesn't have Alice's key:
+    assert_eq!(bob_alice_chat.can_send(bob).await?, false);
+    assert_eq!(bob_alice_chat.id, bob_chat_id);
+
+    let request = bob.pop_sent_msg().await;
+
+    bob_chat_id.delete(bob).await?;
+
+    alice.recv_msg_trash(&request).await;
+    let auth_required = alice.pop_sent_msg().await;
+
+    bob.recv_msg_trash(&auth_required).await;
+
+    // The chat with Alice should be recreated,
+    // and it should be sendable now:
+    assert!(bob.get_chat(alice).await.can_send(bob).await?);
+
+    Ok(())
+}

src/test_utils.rs

@@ -896,6 +896,15 @@ impl TestContext {
     /// If the contact does not exist yet, a new contact will be created
     /// with the correct fingerprint, but without the public key.
     pub async fn add_or_lookup_contact_no_key(&self, other: &TestContext) -> Contact {
+        let contact_id = self.add_or_lookup_contact_id_no_key(other).await;
+        Contact::get_by_id(&self.ctx, contact_id).await.unwrap()
+    }
+
+    /// Returns the [`ContactId`] for the other [`TestContext`], creating it if necessary.
+    ///
+    /// If the contact does not exist yet, a new contact will be created
+    /// with the correct fingerprint, but without the public key.
+    async fn add_or_lookup_contact_id_no_key(&self, other: &TestContext) -> ContactId {
         let primary_self_addr = other.ctx.get_primary_self_addr().await.unwrap();
         let addr = ContactAddress::new(&primary_self_addr).unwrap();
         let fingerprint = self_fingerprint(other).await.unwrap();
@@ -904,7 +913,7 @@ impl TestContext {
             Contact::add_or_lookup_ex(self, "", &addr, fingerprint, Origin::MailinglistAddress)
                 .await
                 .expect("add_or_lookup");
-        Contact::get_by_id(&self.ctx, contact_id).await.unwrap()
+        contact_id
     }
 
     /// Returns 1:1 [`Chat`] with another account address-contact.
@@ -935,7 +944,7 @@ impl TestContext {
     /// so may create a key-contact with a fingerprint
     /// but without the key.
     pub async fn get_chat(&self, other: &TestContext) -> Chat {
-        let contact = self.add_or_lookup_contact_id(other).await;
+        let contact = self.add_or_lookup_contact_id_no_key(other).await;
 
         let chat_id = ChatIdBlocked::lookup_by_contact(&self.ctx, contact)
             .await