Remove unregistered group members

[haffenloher]

Contributor checklist

• I am following the Code Style Guidelines
• I have tested my contribution on these devices:

• Android 6.0 emulator

• My contribution is fully baked and ready to be merged as is
• I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax
• I have made the choice whether I want the BitHub reward or not by omitting or adding the word FREEBIE in the commit message of my first commit

---

Description

Locally remove unregistered users from group membership lists and add a message saying "so and so has left the group." as described in this comment.

Fixes #5891
Fixes #6003

[Trolldemorted]

If you remove a member from a group locally, all subsequent group updates from your client will be ignored, at least on Signal-Desktop clients.

Can we change that behaviour and support kicking by excluding from group update?

[exploide]

Since this got already merged in Signal-Desktop, I guess it is more important to work towards a consistent behavior now.

Currently this is happening: Signal-Desktop removed two unregistered numbers locally, an actual user reinstalled Android, my phone answered to the group info request and hence the two unregistered numbers are back in Signal-Desktop :-P

[Trolldemorted]

When all devices of a group except one notice a member is unregistered and remove him/her, the member re-registers and the last device is coming online, it will still still have the member in the group, but the others do not.

If that previously dormant device is sending a message, the re-registered device(s) will send a group info request to all devices of the sender, receive potentially diverging group infos, and since the group info response is synced to siblings the group will be awfully async:

• the re-registered devices will think they are in the group
• the dormant device which woke up (and all its siblings) will think the re-registered devices are in the group
• all others will not think so

Leading to the re-registered user receiving messages from the dormant device's owner only. Now think of multiple dormant devices, multiple unregistered users and the amount of confusion this will bring upon non tech/signal-savvy users - imho this is way worse than the previous behaviour.

[moxie0]

I just spoke with @liliakai and we decided to revert the change on desktop. Locally removing users can cause lots of synchronization issues (as mentioned here), and we want the behavior to be that no error is shown but that the user is still considered a group member. I think that is what happens on Android now (correct me if I'm wrong), and if that wasn't the case on Desktop then we should resolve whatever bug was occurring there too.

[Trolldemorted]

@moxie0 Android currently sends them until you run into rate limits for pre-keys of an unregistered user, which happens rather frequently: #6003 (comment), signalapp/libsignal-service-java#31

Also every message is not synced to sibling devices: #4859, signalapp/libsignal-service-java#27

[haffenloher]

no error is shown but that the user is still considered a group member

Yep, that's what is happening on Android (edit: not always, see next post).
Still, after sending ~4 messages in quick succdession, the rate limit for fetching prekeys for the unregistered user kicks in. Due to this,

• messages to some users are lost if they're behind the unregistered member in the group membership list (because a RateLimitException aborts the sending process)
• messages to other members are sent five times (the duplicates are now ignored on Android)

My other attempt to work around these issues was signalapp/libsignal-service-java#31 (i.e. ignore the RateLimitExceptions and continue sending to the rest of the group).

[haffenloher]

not completely sure about this, but I think messages actually still get marked as failed on RateLimitExceptions, without ability to resend? #5891 seems to confirm this

[Trolldemorted]

@haffenloher yeah, every exception except a NetworkFailureException is just ignored: https://github.com/WhisperSystems/Signal-Android/blob/9148b7da5f4604dae1106d3932bfa7d3bfa63ad4/src/org/thoughtcrime/securesms/notifications/MessageNotifier.java#L317

[moxie0]

I see what you're saying now, the rate limit check is applied before the user existence check when fetching prekeys. I've done a server deploy which reverses that, so this should no longer be an issue and should not require any client-side changes.

[haffenloher]

Okay, cool! That's indeed a simpler solution =)

[Trolldemorted]

👍

Nevertheless, #4859 is still present - can't we send sync messages immediately?

[wizardofid]

:-o I hoped this would lead to some kind of a solution for one of my repeatedly occuring problems:

groupmember: 'Hi Wiz, i've got a new cellphone and a new number. Please add it to our group and remove the old mumber.'
me: 'I cannot remove your old number. Please simply use the 3-dot-menu to leave the group.'
groupmember: 'Sorry, contract ended and i don't have access to this number any longer.'

Often people also don't de-register. But i think there's a timeout in the server that de-registers numbers after 6 months without contact.
Nevertheless this leads to a bunch of zombie-accounts in our groups. After 6 months the old number can and will be recycled by the telco for another user. If this new user registers for Signal this will lead to a stranger becoming member of our group.
In Germany there's a bad habit to get new phones by signing new contracts, often with a different provider. So most numbers are assigned only for about 2 years to a specific user.

[moxie0]

@wizardofid I agree this is a problem, we're thinking about ways to reengineer the whole groups thing

[Trolldemorted]

@moxie0 how would you like single point-of-truth-groups with one owner device and a logical clock? Non-admin clients could send a group update proposal to the owner, which accepts/declines/autoaccepts the proposal.

This would prevent a group from updating when the owner device is offline or went south, but i think that is better than the status quo, where groups go async quite frequently (e.g. if your group contains an unregistered user, any group update you send will not arrive at your slaves due to #4859).