Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-04-28 02:46:29 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix: Remove group member locally even if send_msg() fails (#5508)

Browse Source 
Otherwise it's impossible to remove a member with missing key from a protected group. In the worst
case a removed member will be added back due to the group membership consistency algo.
This commit is contained in:
iequidoo
2024-05-30 16:57:22 -03:00
committed by iequidoo
parent 980bab3040
commit 24a06d175e
6 changed files with 49 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
src/chat.rs
View File

@@ -3916,8 +3916,7 @@ pub async fn remove_contact_from_chat(
if let Some(contact) = Contact::get_by_id_optional(context, contact_id).await? {
if chat.typ == Chattype::Group && chat.is_promoted() {
msg.viewtype = Viewtype::Text;
if contact.id == ContactId::SELF {
set_group_explicitly_left(context, &chat.grpid).await?;
if contact_id == ContactId::SELF {
msg.text = stock_str::msg_group_left_local(context, ContactId::SELF).await;
} else {
msg.text = stock_str::msg_del_member_local(
@@ -3929,17 +3928,24 @@ pub async fn remove_contact_from_chat(
}
msg.param.set_cmd(SystemMessage::MemberRemovedFromGroup);
msg.param.set(Param::Arg, contact.get_addr().to_lowercase());
msg.id = send_msg(context, chat_id, &mut msg).await?;
let res = send_msg(context, chat_id, &mut msg).await;
if contact_id == ContactId::SELF {
res?;
set_group_explicitly_left(context, &chat.grpid).await?;
} else if let Err(e) = res {
warn!(context, "remove_contact_from_chat({chat_id}, {contact_id}): send_msg() failed: {e:#}.");
}
} else {
sync = Sync;
}
}
// we remove the member from the chat after constructing the
// to-be-send message. If between send_msg() and here the
// process dies the user will have to re-do the action. It's
// better than the other way round: you removed
// someone from DB but no peer or device gets to know about it and
// group membership is thus different on different devices.
// process dies, the user will be able to redo the action. It's better than the other
// way round: you removed someone from DB but no peer or device gets to know about it
// and group membership is thus different on different devices. But if send_msg()
// failed, we still remove the member locally, otherwise it would be impossible to
// remove a member with missing key from a protected group.
// Note also that sending a message needs all recipients
// in order to correctly determine encryption so if we
// removed it first, it would complicate the
@@ -5023,6 +5029,7 @@ mod tests {
// Bob leaves the chat.
remove_contact_from_chat(&bob, bob_chat_id, ContactId::SELF).await?;
bob.pop_sent_msg().await;
// Bob receives a msg about Alice adding Claire to the group.
bob.recv_msg(&alice_sent_add_msg).await;
@@ -5075,6 +5082,7 @@ mod tests {
let sent_msg = alice.pop_sent_msg().await;
bob.recv_msg(&sent_msg).await;
remove_contact_from_chat(&bob, bob_chat_id, bob_fiona_contact_id).await?;
bob.pop_sent_msg().await;
// This doesn't add Fiona back because Bob just removed them.
let sent_msg = alice.send_text(alice_chat_id, "Welcome, Fiona!").await;

30
src/receive_imf/tests.rs
View File

@@ -3923,6 +3923,7 @@ async fn test_dont_readd_with_normal_msg() -> Result<()> {
bob_chat_id.accept(&bob).await?;
remove_contact_from_chat(&bob, bob_chat_id, ContactId::SELF).await?;
bob.pop_sent_msg().await;
assert_eq!(get_chat_contacts(&bob, bob_chat_id).await?.len(), 1);
SystemTime::shift(Duration::from_secs(3600));
@@ -4085,6 +4086,7 @@ async fn test_mua_can_readd() -> Result<()> {
// And leaves it.
remove_contact_from_chat(&alice, alice_chat.id, ContactId::SELF).await?;
alice.pop_sent_msg().await;
assert!(!is_contact_in_chat(&alice, alice_chat.id, ContactId::SELF).await?);
// Bob uses a classical MUA to answer, adding Alice back.
@@ -4159,6 +4161,7 @@ async fn test_recreate_member_list_on_missing_add_of_self() -> Result<()> {
// But if Bob just left, they mustn't recreate the member list even after missing a message.
bob_chat_id.accept(&bob).await?;
remove_contact_from_chat(&bob, bob_chat_id, ContactId::SELF).await?;
bob.pop_sent_msg().await;
send_text_msg(&alice, alice_chat_id, "3rd message".to_string()).await?;
alice.pop_sent_msg().await;
send_text_msg(&alice, alice_chat_id, "4th message".to_string()).await?;
@@ -4498,6 +4501,33 @@ async fn test_leave_protected_group_missing_member_key() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_protected_group_remove_member_missing_key() -> Result<()> {
let mut tcm = TestContextManager::new();
let alice = &tcm.alice().await;
let bob = &tcm.bob().await;
let bob_addr = bob.get_config(Config::Addr).await?.unwrap();
mark_as_verified(alice, bob).await;
let group_id = create_group_chat(alice, ProtectionStatus::Protected, "Group").await?;
let alice_bob_id = alice.add_or_lookup_contact(bob).await.id;
add_contact_to_chat(alice, group_id, alice_bob_id).await?;
alice.send_text(group_id, "Hello!").await;
alice
.sql
.execute("DELETE FROM acpeerstates WHERE addr=?", (&bob_addr,))
.await?;
remove_contact_from_chat(alice, group_id, alice_bob_id).await?;
assert!(!is_contact_in_chat(alice, group_id, alice_bob_id).await?);
let msg = alice.get_last_msg_in(group_id).await;
assert!(msg.is_info());
assert_eq!(
msg.get_text(),
stock_str::msg_del_member_local(alice, &bob_addr, ContactId::SELF,).await
);
assert!(msg.error().is_some());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_forged_from() -> Result<()> {
let mut tcm = TestContextManager::new();

1
src/securejoin.rs
View File

@@ -1403,6 +1403,7 @@ First thread."#;
let alice_bob_contact_id = Contact::create(&alice, "Bob", "bob@example.net").await?;
remove_contact_from_chat(&alice, alice_chat_id, alice_bob_contact_id).await?;
alice.pop_sent_msg().await;
// The message from Bob is delivered late, Bob is already removed.
let msg = alice.recv_msg(&sent).await;

1
src/webxdc.rs
View File

@@ -2588,6 +2588,7 @@ sth_for_the = "future""#
);
remove_contact_from_chat(&alice, chat_id, contact_bob).await?;
alice.pop_sent_msg().await;
let status =
helper_send_receive_status_update(&bob, &alice, &bob_instance, &instance).await?;

2
test-data/golden/chat_test_msg_with_implicit_member_add
View File

@@ -2,7 +2,7 @@ Group#Chat#10: Group chat [3 member(s)]
--------------------------------------------------------------------------------
Msg#10: (Contact#Contact#11): I created a group [FRESH]
Msg#11: (Contact#Contact#11): Member Fiona (fiona@example.net) added by alice@example.org. [FRESH][INFO]
Msg#12: Me (Contact#Contact#Self): You removed member Fiona (fiona@example.net). [INFO] o
Msg#12: Me (Contact#Contact#Self): You removed member Fiona (fiona@example.net). [INFO] √
Msg#13: (Contact#Contact#11): Welcome, Fiona! [FRESH]
Msg#14: info (Contact#Contact#Info): Member Fiona (fiona@example.net) added. [NOTICED][INFO]
Msg#15: (Contact#Contact#11): Welcome back, Fiona! [FRESH]

2
test-data/golden/chat_test_parallel_member_remove
View File

@@ -1,7 +1,7 @@
Group#Chat#10: Group chat [4 member(s)]
--------------------------------------------------------------------------------
Msg#10: (Contact#Contact#10): Hi! I created a group. [FRESH]
Msg#11: Me (Contact#Contact#Self): You left the group. [INFO] o
Msg#11: Me (Contact#Contact#Self): You left the group. [INFO] √
Msg#12: (Contact#Contact#10): Member claire@example.net added by alice@example.org. [FRESH][INFO]
Msg#13: info (Contact#Contact#Info): Member Me (bob@example.net) added. [NOTICED][INFO]
Msg#14: (Contact#Contact#10): What a silence! [FRESH]