fix: Switch to original Hocuri's group membership consistency algo (#3782)(#4624)

- If we don't know the parent (=In-Reply-To) message, then completely recreate the group member list
  (i.e. use the member list of the incoming message) (because we assume that we missed some messages
  & have a wrong group state).
- If the message has a "Chat-Group-Member-Removed: member@example.com" header, then remove this
  member.
- If the message has a "Chat-Group-Member-Added: member@example.com" header, then add this member.

That means:
- Remove checks for the presense of `ContactId::SELF` in the group. Thus all recipients of a message
  take the same decision about group membership changes, no matter if they are in the group
  currently. This fixes a situation when a recipient thinks it's not a member because it missed a
  message about its addition before.
  NOTE: But always recreate membership list if SELF has been added. The older versions of DC don't
  always set "In-Reply-To" to the latest message they sent, but to the latest delivered message (so
  it's a race), so we need this heuristic currently.
- Recreate the group member list if we don't know the parent (=In-Reply-To) message, even if the
  sender isn't in the group as per our view, because we missed some messages and our view may be
  stale.
This commit is contained in:
iequidoo
2023-08-31 16:04:34 -03:00
committed by iequidoo
parent 212fbc125c
commit e12e026bd8
2 changed files with 104 additions and 59 deletions

View File

@@ -1654,40 +1654,27 @@ async fn apply_group_changes(
false
};
// Whether to allow any changes to the member list at all.
let allow_member_list_changes = if chat::is_contact_in_chat(context, chat_id, ContactId::SELF)
.await?
|| self_added
|| !mime_parser.has_chat_version()
{
// Reject old group changes.
chat_id
.update_timestamp(context, Param::MemberListTimestamp, sent_timestamp)
.await?
} else {
// Member list changes are not allowed if we're not in the group
// and are not explicitly added.
// This message comes from a Delta Chat that restored an old backup
// or the message is a MUA reply to an old message.
false
};
// Whether to allow any changes to the member list at all. Just reject old group changes.
let allow_member_list_changes = chat_id
.update_timestamp(context, Param::MemberListTimestamp, sent_timestamp)
.await?;
let is_from_in_chat = chat::is_contact_in_chat(context, chat_id, from_id).await?;
// Whether to rebuild the member list from scratch.
let recreate_member_list = if allow_member_list_changes {
let recreate_member_list = allow_member_list_changes && {
// Recreate member list if the message comes from a MUA as these messages do _not_ set add/remove headers.
// Always recreate membership list if self has been added.
if !mime_parser.has_chat_version() || self_added {
true
} else {
match mime_parser.get_header(HeaderDef::InReplyTo) {
(is_from_in_chat && !mime_parser.has_chat_version())
// Always recreate membership list if SELF has been added. The older versions of DC
// don't always set "In-Reply-To" to the latest message they sent, but to the latest
// delivered message (so it's a race), so we have this heuristic here.
|| self_added
|| match mime_parser.get_header(HeaderDef::InReplyTo) {
// If we don't know the referenced message, we missed some messages.
// Maybe they added/removed members, so we need to recreate our member list.
Some(reply_to) => rfc724_mid_exists(context, reply_to).await?.is_none(),
None => false,
}
}
} else {
false
};
if let Some(removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) {
@@ -1794,43 +1781,41 @@ async fn apply_group_changes(
// Recreate the member list.
if recreate_member_list {
if !chat::is_contact_in_chat(context, chat_id, from_id).await? {
if !is_from_in_chat {
warn!(
context,
"Contact {from_id} attempts to modify group chat {chat_id} member list without being a member."
"Contact {from_id} modifies group chat {chat_id} member list possibly not being a member."
);
} else {
// Only delete old contacts if the sender is not a classical MUA user:
// Classical MUA users usually don't intend to remove users from an email
// thread, so if they removed a recipient then it was probably by accident.
if mime_parser.has_chat_version() {
context
.sql
.execute("DELETE FROM chats_contacts WHERE chat_id=?;", (chat_id,))
.await?;
}
let mut members_to_add = HashSet::new();
members_to_add.extend(to_ids);
members_to_add.insert(ContactId::SELF);
if !from_id.is_special() {
members_to_add.insert(from_id);
}
if let Some(removed_id) = removed_id {
members_to_add.remove(&removed_id);
}
info!(
context,
"Recreating chat {chat_id} with members {members_to_add:?}."
);
chat::add_to_chat_contacts_table(context, chat_id, &Vec::from_iter(members_to_add))
.await?;
send_event_chat_modified = true;
}
// Only delete old contacts if the sender is not a classical MUA user:
// Classical MUA users usually don't intend to remove users from an email
// thread, so if they removed a recipient then it was probably by accident.
if mime_parser.has_chat_version() {
context
.sql
.execute("DELETE FROM chats_contacts WHERE chat_id=?;", (chat_id,))
.await?;
}
let mut members_to_add = HashSet::new();
members_to_add.extend(to_ids);
members_to_add.insert(ContactId::SELF);
if !from_id.is_special() {
members_to_add.insert(from_id);
}
if let Some(removed_id) = removed_id {
members_to_add.remove(&removed_id);
}
info!(
context,
"Recreating chat {chat_id} with members {members_to_add:?}."
);
chat::add_to_chat_contacts_table(context, chat_id, &Vec::from_iter(members_to_add)).await?;
send_event_chat_modified = true;
}
if let Some(avatar_action) = &mime_parser.group_avatar {