Skip to content

Commit bf72b3a

Browse files
committed
fix: Remove SecurejoinWait info message when received Alice's key (#7585)
And don't add a `SecurejoinWait` info message at all if we know Alice's key from the start. If we don't remove this info message, it appears in the chat after "Messages are end-to-end encrypted..." which is quite confusing when Bob can already send messages to Alice.
1 parent 30f2981 commit bf72b3a

File tree

2 files changed

+78
-31
lines changed

2 files changed

+78
-31
lines changed

src/securejoin/bob.rs

Lines changed: 65 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,16 @@ use anyhow::{Context as _, Result};
55
use super::HandshakeMessage;
66
use super::qrinvite::QrInvite;
77
use crate::chat::{self, ChatId, is_contact_in_chat};
8+
use crate::chatlist_events;
89
use crate::constants::{Blocked, Chattype};
910
use crate::contact::Origin;
1011
use crate::context::Context;
1112
use crate::events::EventType;
1213
use crate::key::self_fingerprint;
13-
use crate::message::{Message, Viewtype};
14+
use crate::log::LogExt;
15+
use crate::message::{Message, MsgId, Viewtype};
1416
use crate::mimeparser::{MimeMessage, SystemMessage};
15-
use crate::param::Param;
17+
use crate::param::{Param, Params};
1618
use crate::securejoin::{ContactId, encrypted_and_signed, verify_sender_by_fingerprint};
1719
use crate::stock_str;
1820
use crate::sync::Sync::*;
@@ -48,16 +50,16 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul
4850
ContactId::scaleup_origin(context, &[invite.contact_id()], Origin::SecurejoinJoined).await?;
4951
context.emit_event(EventType::ContactsChanged(None));
5052

53+
let has_key = context
54+
.sql
55+
.exists(
56+
"SELECT COUNT(*) FROM public_keys WHERE fingerprint=?",
57+
(invite.fingerprint().hex(),),
58+
)
59+
.await?;
60+
5161
// Now start the protocol and initialise the state.
5262
{
53-
let has_key = context
54-
.sql
55-
.exists(
56-
"SELECT COUNT(*) FROM public_keys WHERE fingerprint=?",
57-
(invite.fingerprint().hex(),),
58-
)
59-
.await?;
60-
6163
// `joining_chat_id` is `Some` if group chat
6264
// already exists and we are in the chat.
6365
let joining_chat_id = match invite {
@@ -142,20 +144,22 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul
142144
Ok(joining_chat_id)
143145
}
144146
QrInvite::Contact { .. } => {
145-
// For setup-contact the BobState already ensured the 1:1 chat exists because it
146-
// uses it to send the handshake messages.
147-
chat::add_info_msg_with_cmd(
148-
context,
149-
private_chat_id,
150-
&stock_str::securejoin_wait(context).await,
151-
SystemMessage::SecurejoinWait,
152-
None,
153-
time(),
154-
None,
155-
None,
156-
None,
157-
)
158-
.await?;
147+
// For setup-contact the BobState already ensured the 1:1 chat exists because it is
148+
// used to send the handshake messages.
149+
if !has_key {
150+
chat::add_info_msg_with_cmd(
151+
context,
152+
private_chat_id,
153+
&stock_str::securejoin_wait(context).await,
154+
SystemMessage::SecurejoinWait,
155+
None,
156+
time(),
157+
None,
158+
None,
159+
None,
160+
)
161+
.await?;
162+
}
159163
Ok(private_chat_id)
160164
}
161165
}
@@ -177,6 +181,38 @@ async fn insert_new_db_entry(context: &Context, invite: QrInvite, chat_id: ChatI
177181
.await
178182
}
179183

184+
async fn delete_securejoin_wait_msg(context: &Context, chat_id: ChatId) -> Result<()> {
185+
if let Some((msg_id, param)) = context
186+
.sql
187+
.query_row_optional(
188+
"
189+
SELECT id, param FROM msgs
190+
WHERE timestamp=(SELECT MAX(timestamp) FROM msgs WHERE chat_id=? AND hidden=0)
191+
AND chat_id=? AND hidden=0
192+
LIMIT 1
193+
",
194+
(chat_id, chat_id),
195+
|row| {
196+
let id: MsgId = row.get(0)?;
197+
let param: String = row.get(1)?;
198+
let param: Params = param.parse().unwrap_or_default();
199+
Ok((id, param))
200+
},
201+
)
202+
.await?
203+
&& param.get_cmd() == SystemMessage::SecurejoinWait
204+
{
205+
let on_server = false;
206+
msg_id.trash(context, on_server).await?;
207+
context.emit_event(EventType::MsgDeleted { chat_id, msg_id });
208+
context.emit_msgs_changed_without_msg_id(chat_id);
209+
chatlist_events::emit_chatlist_item_changed(context, chat_id);
210+
context.emit_msgs_changed_without_ids();
211+
chatlist_events::emit_chatlist_changed(context);
212+
}
213+
Ok(())
214+
}
215+
180216
/// Handles `vc-auth-required` and `vg-auth-required` handshake messages.
181217
///
182218
/// # Bob - the joiner's side
@@ -213,6 +249,11 @@ pub(super) async fn handle_auth_required(
213249

214250
info!(context, "Fingerprint verified.",);
215251
let chat_id = private_chat_id(context, &invite).await?;
252+
delete_securejoin_wait_msg(context, chat_id)
253+
.await
254+
.context("delete_securejoin_wait_msg")
255+
.log_err(context)
256+
.ok();
216257
send_handshake_message(context, &invite, chat_id, BobHandshakeMsg::RequestWithAuth).await?;
217258
context
218259
.sql

src/securejoin/securejoin_tests.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,17 @@ async fn test_setup_contact_ex(case: SetupContactCase) {
100100
bob_chat.why_cant_send(&bob).await.unwrap(),
101101
Some(CantSendReason::MissingKey)
102102
);
103+
104+
// Check Bob's info messages.
105+
let msg_cnt = 2;
106+
let mut i = 0..msg_cnt;
107+
let msg = get_chat_msg(&bob, bob_chat.get_id(), i.next().unwrap(), msg_cnt).await;
108+
assert!(msg.is_info());
109+
assert_eq!(msg.get_text(), messages_e2e_encrypted(&bob).await);
110+
let msg = get_chat_msg(&bob, bob_chat.get_id(), i.next().unwrap(), msg_cnt).await;
111+
assert!(msg.is_info());
112+
assert_eq!(msg.get_text(), stock_str::securejoin_wait(&bob).await);
113+
103114
let contact_alice_id = bob.add_or_lookup_contact_no_key(&alice).await.id;
104115
let sent = bob.pop_sent_msg().await;
105116
assert!(!sent.payload.contains("Bob Examplenet"));
@@ -272,15 +283,10 @@ async fn test_setup_contact_ex(case: SetupContactCase) {
272283
assert!(contact_alice.get_name().is_empty());
273284
assert_eq!(contact_alice.is_bot(), case == SetupContactCase::AliceIsBot);
274285

275-
// Check Bob got expected info messages in his 1:1 chat.
276-
let msg_cnt = 2;
277-
let mut i = 0..msg_cnt;
278-
let msg = get_chat_msg(&bob, bob_chat.get_id(), i.next().unwrap(), msg_cnt).await;
286+
// The `SecurejoinWait` info message has been removed, but the e2ee notice remains.
287+
let msg = get_chat_msg(&bob, bob_chat.get_id(), 0, 1).await;
279288
assert!(msg.is_info());
280289
assert_eq!(msg.get_text(), messages_e2e_encrypted(&bob).await);
281-
let msg = get_chat_msg(&bob, bob_chat.get_id(), i.next().unwrap(), msg_cnt).await;
282-
assert!(msg.is_info());
283-
assert_eq!(msg.get_text(), stock_str::securejoin_wait(&bob).await);
284290
}
285291

286292
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]

0 commit comments

Comments
 (0)