Skip to content

Commit 4968f72

Browse files
authored
fix permanently hiding of one-to-one chats after secure-join (#2791)
* test one-to-one chats on setup-contact/secure-join only one chat is created after scanning a QR code: - on setup-contact, one-to-ones are created on both sided - on secure-join, the joined group chat is created; one-to-ones are not created intitally, but should become visible on receiving messages * make sure, Alice creates the chat with Bob on setup-contact not totally sure if that change in #2508 was on-purpose, however, all yet released versions did create the one-to-one chat also on the Inviter's (Alice) side, so, let's stay with that, i do not see many reasons to change that. * unblock hidden (Blocked::Yes) one-to-one chats one-to-one chats may be hidden by secure-join, in case someone later writes a message to it (not unlikely), the chat needs to be shown. before, messages are just not shown, the corresponding chat did not appear. the 'Blocked' wording of a 'Chat' must not be mixed with the 'Blocking' of a contact. 'Chat-Blocking' is mostly a visibility thing, that may change as messages come in. this change should not affect _really_ blocked contacts - they are filtered out already before and their messages are usually not even downloaded. also, before allow_creation is checked, that may disallow chat creation for show_emails reasons. all in all, it just does the same as if the user has manualy deleted the chat before and it would be created. * simplify test
1 parent b24a0ed commit 4968f72

File tree

3 files changed

+39
-16
lines changed

3 files changed

+39
-16
lines changed

src/chat.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ impl ChatId {
266266
/// Updates chat blocked status.
267267
///
268268
/// Returns true if the value was modified.
269-
async fn set_blocked(self, context: &Context, new_blocked: Blocked) -> Result<bool> {
269+
pub(crate) async fn set_blocked(self, context: &Context, new_blocked: Blocked) -> Result<bool> {
270270
if self.is_special() {
271271
bail!("ignoring setting of Block-status for {}", self);
272272
}

src/dc_receive_imf.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -664,11 +664,12 @@ async fn add_parts(
664664
}
665665

666666
if let Some(chat_id) = chat_id {
667-
if Blocked::Not != chat_id_blocked {
668-
if Blocked::Not == create_blocked {
669-
chat_id.unblock(context).await?;
670-
chat_id_blocked = Blocked::Not;
671-
} else if parent.is_some() {
667+
if chat_id_blocked != Blocked::Not {
668+
if chat_id_blocked != create_blocked {
669+
chat_id.set_blocked(context, create_blocked).await?;
670+
chat_id_blocked = create_blocked;
671+
}
672+
if create_blocked == Blocked::Request && parent.is_some() {
672673
// we do not want any chat to be created implicitly. Because of the origin-scale-up,
673674
// the contact requests will pop up and this should be just fine.
674675
Contact::scaleup_origin_by_id(context, from_id, Origin::IncomingReplyTo)
@@ -806,9 +807,9 @@ async fn add_parts(
806807
}
807808

808809
if let Some(chat_id) = chat_id {
809-
if Blocked::Not != chat_id_blocked && Blocked::Not == create_blocked {
810-
chat_id.unblock(context).await?;
811-
chat_id_blocked = Blocked::Not;
810+
if chat_id_blocked != Blocked::Not && chat_id_blocked != create_blocked {
811+
chat_id.set_blocked(context, create_blocked).await?;
812+
chat_id_blocked = create_blocked;
812813
}
813814
}
814815
}

src/securejoin.rs

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,12 @@ pub(crate) async fn handle_securejoin_handshake(
499499

500500
inviter_progress!(context, contact_id, 300);
501501

502+
// for setup-contact, make Alice's one-to-one chat with Bob visible
503+
// (secure-join-information are shown in the group chat)
504+
if !join_vg {
505+
ChatId::create_for_contact(context, contact_id).await?;
506+
}
507+
502508
// Alice -> Bob
503509
send_alice_handshake_msg(
504510
context,
@@ -936,6 +942,8 @@ mod tests {
936942
async fn test_setup_contact() -> Result<()> {
937943
let alice = TestContext::new_alice().await;
938944
let bob = TestContext::new_bob().await;
945+
assert_eq!(Chatlist::try_load(&alice, 0, None, None).await?.len(), 0);
946+
assert_eq!(Chatlist::try_load(&bob, 0, None, None).await?.len(), 0);
939947

940948
// Setup JoinerProgress sinks.
941949
let (joiner_progress_tx, joiner_progress_rx) = async_std::channel::bounded(100);
@@ -954,6 +962,7 @@ mod tests {
954962

955963
// Step 2: Bob scans QR-code, sends vc-request
956964
dc_join_securejoin(&bob.ctx, &qr).await?;
965+
assert_eq!(Chatlist::try_load(&bob, 0, None, None).await?.len(), 1);
957966

958967
let sent = bob.pop_sent_msg().await;
959968
assert!(!bob.ctx.has_ongoing().await);
@@ -965,6 +974,7 @@ mod tests {
965974

966975
// Step 3: Alice receives vc-request, sends vc-auth-required
967976
alice.recv_msg(&sent).await;
977+
assert_eq!(Chatlist::try_load(&alice, 0, None, None).await?.len(), 1);
968978

969979
let sent = alice.pop_sent_msg().await;
970980
let msg = bob.parse_msg(&sent).await;
@@ -1041,6 +1051,11 @@ mod tests {
10411051
VerifiedStatus::BidirectVerified
10421052
);
10431053

1054+
// exactly one one-to-one chat should be visible for both now
1055+
// (check this before calling alice.create_chat() explicitly below)
1056+
assert_eq!(Chatlist::try_load(&alice, 0, None, None).await?.len(), 1);
1057+
assert_eq!(Chatlist::try_load(&bob, 0, None, None).await?.len(), 1);
1058+
10441059
// Check Alice got the verified message in her 1:1 chat.
10451060
{
10461061
let chat = alice.create_chat(&bob).await;
@@ -1302,6 +1317,8 @@ mod tests {
13021317
async fn test_secure_join() -> Result<()> {
13031318
let alice = TestContext::new_alice().await;
13041319
let bob = TestContext::new_bob().await;
1320+
assert_eq!(Chatlist::try_load(&alice, 0, None, None).await?.len(), 0);
1321+
assert_eq!(Chatlist::try_load(&bob, 0, None, None).await?.len(), 0);
13051322

13061323
// Setup JoinerProgress sinks.
13071324
let (joiner_progress_tx, joiner_progress_rx) = async_std::channel::bounded(100);
@@ -1323,12 +1340,9 @@ mod tests {
13231340
.await
13241341
.unwrap();
13251342

1326-
// Step 2: Bob scans QR-code, sends vg-request; blocks on ongoing process
1327-
let joiner = {
1328-
let qr = qr.clone();
1329-
let ctx = bob.ctx.clone();
1330-
async_std::task::spawn(async move { dc_join_securejoin(&ctx, &qr).await.unwrap() })
1331-
};
1343+
// Step 2: Bob scans QR-code, sends vg-request
1344+
let bob_chatid = dc_join_securejoin(&bob.ctx, &qr).await?;
1345+
assert_eq!(Chatlist::try_load(&bob, 0, None, None).await?.len(), 1);
13321346

13331347
let sent = bob.pop_sent_msg().await;
13341348
assert_eq!(sent.recipient(), "[email protected]".parse().unwrap());
@@ -1444,16 +1458,24 @@ mod tests {
14441458
"vg-member-added-received"
14451459
);
14461460

1447-
let bob_chatid = joiner.await;
14481461
let bob_chat = Chat::load_from_db(&bob.ctx, bob_chatid).await?;
14491462
assert!(bob_chat.is_protected());
1463+
assert!(bob_chat.typ == Chattype::Group);
14501464
assert!(!bob.ctx.has_ongoing().await);
14511465

14521466
// On this "happy path", Alice and Bob get only a group-chat where all information are added to.
14531467
// The one-to-one chats are used internally for the hidden handshake messages,
14541468
// however, should not be visible in the UIs.
14551469
assert_eq!(Chatlist::try_load(&alice, 0, None, None).await?.len(), 1);
14561470
assert_eq!(Chatlist::try_load(&bob, 0, None, None).await?.len(), 1);
1471+
1472+
// If Bob then sends a direct message to alice, however, the one-to-one with Alice should appear.
1473+
let bobs_chat_with_alice = bob.create_chat(&alice).await;
1474+
let sent = bob.send_text(bobs_chat_with_alice.id, "Hello").await;
1475+
alice.recv_msg(&sent).await;
1476+
assert_eq!(Chatlist::try_load(&alice, 0, None, None).await?.len(), 2);
1477+
assert_eq!(Chatlist::try_load(&bob, 0, None, None).await?.len(), 2);
1478+
14571479
Ok(())
14581480
}
14591481
}

0 commit comments

Comments
 (0)