Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Users are implicitly removed without any visible message #4934

Closed
link2xt opened this issue Nov 5, 2023 · 13 comments · Fixed by #4938
Closed

Users are implicitly removed without any visible message #4934

link2xt opened this issue Nov 5, 2023 · 13 comments · Fixed by #4938
Assignees
Labels
bug Something is not working

Comments

@link2xt
Copy link
Collaborator

link2xt commented Nov 5, 2023

We currently warn about this in the log, but not in the chat:

warn!(context, "Implicit removal of {diff:?} from chat {chat_id}.");

Some message indicating that we have probably missed a "Member removed" or the other client is buggy should be added to the chat, but not blaming the user of the incoming message as they may actually be not the one who removed the member from the member list.

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 5, 2023

Implicit removal generally indicates that someone lost a message or not processed it due to a bug like #4570, so at least this would add a chance to notice.

@brabo
Copy link

brabo commented Nov 5, 2023

I think this is already a good step. For context: I noticed in a group that I was unable to see link2xt's messages, whereupon link2xt re-added me. It seems an implicit removal caused his client to remove me from his member list for that group.

@link2xt link2xt added the bug Something is not working label Nov 5, 2023
@link2xt
Copy link
Collaborator Author

link2xt commented Nov 5, 2023

cc @iequidoo Another option is that we never remove members implicitly but only add.

Removal is problematic because the member who gets removed never notices it and keeps writing to the group, then messages appear with ~ in the beginning of the display name and someone has to add the member back and maybe properly remove if they want the member to actually be removed.

An advantage is also no need for a new string to translate.

@brabo
Copy link

brabo commented Nov 5, 2023

Or not do it based on 1 other client, but go for correcting the 1 wrong client based on a number of other clients?
Wether it is to add or to remove, both could be done that way without causing issues for some members?

Because just adding without checks would effectively re-add an removed member in case of a lost explicit removal message?

@iequidoo
Copy link
Collaborator

iequidoo commented Nov 6, 2023

cc @iequidoo Another option is that we never remove members implicitly but only add.

I thought that removal is better for privacy than adding absent members locally, but this is a debatable thing anyway.

Removal is problematic because the member who gets removed never notices it and keeps writing to the group, then messages appear with ~ in the beginning of the display name and someone has to add the member back and maybe properly remove if they want the member to actually be removed.

This is the real problem, yes. I even think it's a bigger problem that the subject one.

An advantage is also no need for a new string to translate.

But then we should add a message that a new member (or several of them) were added to the group by ... whom? The user should be aware of who receives their messages. This is not mandatory though because race conditions are possible anyway when the user sends a message. But at least they would know eventually. Otherwise the user should look into member list on their own to know.

Another option is to allow membership inconsistencies, i.e. neither add ot remove. At least this is the simplest solution. Now i even start to think this is the best option. It even could be a feature -- the user can "unsubscribe" others from their messages.

@iequidoo
Copy link
Collaborator

iequidoo commented Nov 6, 2023

The only problem is that, if we allow membership inconsistencies, further member list changes by those who are absent in the group locally can't be done, and that would lead to even more membership inconsistencies.

@link2xt link2xt changed the title Add system message when a member is implicitly removed Users are implicitly removed without any visible message Nov 6, 2023
@link2xt
Copy link
Collaborator Author

link2xt commented Nov 6, 2023

Another option is to allow membership inconsistencies, i.e. neither add ot remove. At least this is the simplest solution. Now i even start to think this is the best option. It even could be a feature -- the user can "unsubscribe" others from their messages.

We definitely want eventual consistency, this is the reason #4185 started. Chats are generally expected to look the same on all devices.

I propose that we only add members implicitly, but not remove. We already decided that non-DC MUAs should only be able to add users, but we can unify this with DC as well. Then the only way the user can get into the state of being excluded from the member list without knowing it is when they have lost the "Member removed" message mentioning them.

But then we should add a message that a new member (or several of them) were added to the group by ... whom?

If we want to prevent silent adding of new members we can add a notice like "Member %1$s added." without mentioning who added it. This is good enough and should only happen if there are lost or reordered messages. But I would not even consider it being a part of this issue, adding members without a notice was always possible in the old core e.g. by removing someone and triggering a member list rebuild this way. Also in general we don't assume malicious users and malicious users may leak the messages silently anyway without even the need to add anyone.

@iequidoo
Copy link
Collaborator

iequidoo commented Nov 6, 2023

We definitely want eventual consistency, this is the reason #4185 started. Chats are generally expected to look the same on all devices.

On devices of a particular user or on all users' devices? The first can be solved w/o always adding locally absent members implicitly.

I propose that we only add members implicitly, but not remove. We already decided that non-DC MUAs should only be able to add users, but we can unify this with DC as well.

That's a good argumentation. Of course if we're not going to change the behaviour for non-DC MUAs.

If we want to prevent silent adding of new members we can add a notice like "Member %1$s added." without mentioning who added it. This is good enough and should only happen if there are lost or reordered messages. But I would not even consider it being a part of this issue, adding members without a notice was always possible in the old core e.g. by removing someone and triggering a member list rebuild this way. Also in general we don't assume malicious users and malicious users may leak the messages silently anyway without even the need to add anyone.

I worry not about malicious users, but only about that users don't have full information about where their messages go. In general i'd prefer that there are no races possible with new member additions when i send a message. Privacy concerns remain if there are no malicious users at all. In the end, other users don't know which message i'm about to send when they add new members. But i understand that this problem anyway can't be solved easily in the current design.

Ok, i'll implement adding members instead of removing then, but your opinion about my concerns is welcome anyway

@iequidoo iequidoo self-assigned this Nov 6, 2023
@iequidoo
Copy link
Collaborator

iequidoo commented Nov 6, 2023

I want to say, in classical MUAs you have full control over your messages, if you are going to send smth confidential, you can type smth like "Guys, this is a confidential message: ..." and check To/CC/etc. I'd like it is possible in DC too.

UPD: Probably, i just need to create a new group for such a message and then it works. But i think this is not evident for DC users, and it's not convenient if i want it to be a reply to some previous message

iequidoo added a commit that referenced this issue Nov 6, 2023
…d absent ones instead (#4934)

This is another approach to provide group membership consistency for all members. Considerations:
- 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.
- DC users could miss new member additions and then better to handle this in the same way as for
  classical MUA messages. Moreover, if we remove a member implicitly, they will never know that and
  continue to think they're still here.

But it shouldn't be a big problem if somebody missed a member removal, because they will likely
recreate the member list from the next received message. The problem occurs only if that "somebody"
managed to reply earlier. Really, it's a problem for big groups with high message rate, but let it
be for now.
iequidoo added a commit that referenced this issue Nov 6, 2023
…d absent ones instead (#4934)

This is another approach to provide group membership consistency for all members. Considerations:
- 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.
- DC users could miss new member additions and then better to handle this in the same way as for
  classical MUA messages. Moreover, if we remove a member implicitly, they will never know that and
  continue to think they're still here.

But it shouldn't be a big problem if somebody missed a member removal, because they will likely
recreate the member list from the next received message. The problem occurs only if that "somebody"
managed to reply earlier. Really, it's a problem for big groups with high message rate, but let it
be for now.
@link2xt
Copy link
Collaborator Author

link2xt commented Nov 6, 2023

I want to say, in classical MUAs you have full control over your messages, if you are going to send smth confidential, you can type smth like "Guys, this is a confidential message: ..." and check To/CC/etc. I'd like it is possible in DC too.

I think it makes more sense to compare this to different messengers. Does Signal, WhatsApp, Telegram or Wire handle it differently if someone adds a member right before you send a message? Many times users even expect that new members will see the history of the group.

@brabo
Copy link

brabo commented Nov 6, 2023

Adding would have had a more positive effect in this situation. For some reason someone else no longer had me in their member list, and link2xt's core then did a removal based on that. If the other party would have added me back instead, there would not have been 2 people who's messages I was unable to see.

Thanks for giving this care and consideration!

link2xt pushed a commit that referenced this issue Nov 6, 2023
…d absent ones instead (#4934)

This is another approach to provide group membership consistency for all members. Considerations:
- 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.
- DC users could miss new member additions and then better to handle this in the same way as for
  classical MUA messages. Moreover, if we remove a member implicitly, they will never know that and
  continue to think they're still here.

But it shouldn't be a big problem if somebody missed a member removal, because they will likely
recreate the member list from the next received message. The problem occurs only if that "somebody"
managed to reply earlier. Really, it's a problem for big groups with high message rate, but let it
be for now.
@iequidoo
Copy link
Collaborator

iequidoo commented Nov 6, 2023

But i'm still asking, is consistency really needed for different users or only for devices of a particular user? In the latter case @brabo at least would see @link2xt's messages. Generally speaking, brabo would see at least messages of the user who added them, for me it looks good enough and less efforts are needed to achieve that. Yes, in this case a user can't be sure that they see all messages, but as long as there can be mail delivery issues this problem exists anyway.

@link2xt
Copy link
Collaborator Author

link2xt commented Nov 7, 2023

But i'm still asking, is consistency really needed for different users or only for devices of a particular user?

Ideally we want to have eventual consistency of the group state for all users. If for some time no messages are lost and all users are chatting, all devices of all users should get the same group state. Messengers like WhatsApp and Telegram achieve this by managing the group state in a central place and this is what users expect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working
Projects
None yet
3 participants