From f149065a30c8221d05e9df8a44d27ce54a6006b5 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Thu, 9 Apr 2020 13:17:00 +1000 Subject: [PATCH] Fix GroupDescriptions updating dynamically. This shouldn't happen, rather it should have the snapshot of the group state per message. --- .../groups/GroupMessageProcessor.java | 38 ++++++----- .../securesms/util/GroupUtil.java | 63 +++++++------------ 2 files changed, 45 insertions(+), 56 deletions(-) diff --git a/src/org/thoughtcrime/securesms/groups/GroupMessageProcessor.java b/src/org/thoughtcrime/securesms/groups/GroupMessageProcessor.java index 4e53e6273b..719a7dd7d8 100644 --- a/src/org/thoughtcrime/securesms/groups/GroupMessageProcessor.java +++ b/src/org/thoughtcrime/securesms/groups/GroupMessageProcessor.java @@ -137,13 +137,25 @@ public class GroupMessageProcessor { GroupDatabase database = DatabaseFactory.getGroupDatabase(context); String id = GroupUtil.getEncodedId(group); - // Only update group if admin sent the message + String masterHexEncodedPublicKey = TextSecurePreferences.getMasterHexEncodedPublicKey(context); + String ourHexEncodedPublicKey = masterHexEncodedPublicKey != null ? masterHexEncodedPublicKey : TextSecurePreferences.getLocalNumber(context); + if (group.getGroupType() == SignalServiceGroup.GroupType.SIGNAL) { + // Only update group if admin sent the message String hexEncodedPublicKey = getMasterHexEncodedPublicKey(context, content.getSender()); if (!groupRecord.getAdmins().contains(Address.fromSerialized(hexEncodedPublicKey))) { Log.d("Loki - Group Message", "Received a group update message from a non-admin user for " + id +". Ignoring."); return null; } + + // We should only process update message if we were in the group + Address ourAddress = Address.fromSerialized(ourHexEncodedPublicKey); + if (!groupRecord.getMembers().contains(ourAddress) && + !group.getMembers().or(Collections.emptyList()).contains(ourHexEncodedPublicKey)) { + Log.d("Loki - Group Message", "Received a group update message from a group we are not members in: " + id + " . Ignoring."); + database.setActive(id, false); + return null; + } } Set
currentMembers = new HashSet<>(groupRecord.getMembers()); @@ -169,23 +181,14 @@ public class GroupMessageProcessor { database.updateMembers(id, new LinkedList<>(newMembers)); } - builder.clearMembers(); - // We add any new or removed members to the group context // This will allow us later to iterate over them to check if they left or were added for UI display for (Address addedMember : addedMembers) { - builder.addMembers(addedMember.serialize()); + builder.addNewMembers(addedMember.serialize()); } for (Address removedMember : removedMembers) { - builder.addMembers(removedMember.serialize()); - } - - // If we were removed then we need to disable the chat - String masterHexEncodedPublicKey = TextSecurePreferences.getMasterHexEncodedPublicKey(context); - String ourHexEncodedPublicKey = masterHexEncodedPublicKey != null ? masterHexEncodedPublicKey : TextSecurePreferences.getLocalNumber(context); - if (removedMembers.contains(Address.fromSerialized(ourHexEncodedPublicKey))) { - database.setActive(id, false); + builder.addRemovedMembers(removedMember.serialize()); } if (group.getName().isPresent() || group.getAvatar().isPresent()) { @@ -197,10 +200,15 @@ public class GroupMessageProcessor { builder.clearName(); } - if (!groupRecord.isActive()) database.setActive(id, true); + // If we were removed then we need to disable the chat + if (removedMembers.contains(Address.fromSerialized(ourHexEncodedPublicKey))) { + database.setActive(id, false); + } else { + if (!groupRecord.isActive()) database.setActive(id, true); - if (group.getMembers().isPresent()) { - establishSessionsWithMembersIfNeeded(context, group.getMembers().get()); + if (group.getMembers().isPresent()) { + establishSessionsWithMembersIfNeeded(context, group.getMembers().get()); + } } return storeMessage(context, content, group, builder.build(), outgoing); diff --git a/src/org/thoughtcrime/securesms/util/GroupUtil.java b/src/org/thoughtcrime/securesms/util/GroupUtil.java index 907a21217e..0029d5f6a4 100644 --- a/src/org/thoughtcrime/securesms/util/GroupUtil.java +++ b/src/org/thoughtcrime/securesms/util/GroupUtil.java @@ -10,7 +10,6 @@ import com.google.protobuf.ByteString; import org.thoughtcrime.securesms.database.Address; import org.thoughtcrime.securesms.database.DatabaseFactory; import org.thoughtcrime.securesms.database.GroupDatabase; -import org.thoughtcrime.securesms.database.GroupDatabase.GroupRecord; import org.thoughtcrime.securesms.logging.Log; import org.thoughtcrime.securesms.mms.OutgoingGroupMediaMessage; import org.thoughtcrime.securesms.recipients.Recipient; @@ -159,7 +158,7 @@ public class GroupUtil { @NonNull private final Context context; @Nullable private final GroupContext groupContext; - private final List members; + private final List newMembers; private final List removedMembers; private boolean ourDeviceWasRemoved; @@ -167,35 +166,35 @@ public class GroupUtil { this.context = context.getApplicationContext(); this.groupContext = groupContext; - this.members = new LinkedList<>(); + this.newMembers = new LinkedList<>(); this.removedMembers = new LinkedList<>(); this.ourDeviceWasRemoved = false; - if (groupContext != null && !groupContext.getMembersList().isEmpty()) { - List memberList = groupContext.getMembersList(); - List
currentMembers = getCurrentGroupMembers(); + if (groupContext != null) { + List newMembers = groupContext.getNewMembersList(); + for (String member : newMembers) { + this.newMembers.add(this.toRecipient(member)); + } - // Add them to the member or removed members lists - for (String member : memberList) { - Address address = Address.fromSerialized(member); - Recipient recipient = Recipient.from(context, address, true); - if (currentMembers == null || currentMembers.contains(address)) { - this.members.add(recipient); - } else { - this.removedMembers.add(recipient); - } + List removedMembers = groupContext.getRemovedMembersList(); + for (String member : removedMembers) { + this.removedMembers.add(this.toRecipient(member)); } // Check if our device was removed if (!removedMembers.isEmpty()) { String masterHexEncodedPublicKey = TextSecurePreferences.getMasterHexEncodedPublicKey(context); String hexEncodedPublicKey = masterHexEncodedPublicKey != null ? masterHexEncodedPublicKey : TextSecurePreferences.getLocalNumber(context); - Recipient self = Recipient.from(context, Address.fromSerialized(hexEncodedPublicKey), false); - ourDeviceWasRemoved = removedMembers.contains(self); + ourDeviceWasRemoved = removedMembers.contains(hexEncodedPublicKey); } } } + private Recipient toRecipient(String hexEncodedPublicKey) { + Address address = Address.fromSerialized(hexEncodedPublicKey); + return Recipient.from(context, address, false); + } + public String toString(Recipient sender) { // Show the local removed message if (ourDeviceWasRemoved) { @@ -211,10 +210,10 @@ public class GroupUtil { String title = groupContext.getName(); - if (!members.isEmpty()) { + if (!newMembers.isEmpty()) { description.append("\n"); description.append(context.getResources().getQuantityString(R.plurals.GroupUtil_joined_the_group, - members.size(), toString(members))); + newMembers.size(), toString(newMembers))); } if (!removedMembers.isEmpty()) { @@ -224,8 +223,8 @@ public class GroupUtil { } if (title != null && !title.trim().isEmpty()) { - if (!members.isEmpty()) description.append(" "); - else description.append("\n"); + String separator = (!newMembers.isEmpty() || !removedMembers.isEmpty()) ? " " : "\n"; + description.append(separator); description.append(context.getString(R.string.GroupUtil_group_name_is_now, title)); } @@ -233,8 +232,8 @@ public class GroupUtil { } public void addListener(RecipientModifiedListener listener) { - if (!this.members.isEmpty()) { - for (Recipient member : this.members) { + if (!this.newMembers.isEmpty()) { + for (Recipient member : this.newMembers) { member.addListener(listener); } } @@ -252,23 +251,5 @@ public class GroupUtil { return result; } - - private List
getCurrentGroupMembers() { - if (groupContext == null) { return null; } - GroupDatabase groupDatabase = DatabaseFactory.getGroupDatabase(context); - byte[] decodedGroupId = groupContext.getId().toByteArray(); - String signalGroupId = getEncodedId(decodedGroupId, false); - String publicChatId = getEncodedPublicChatId(decodedGroupId); - String rssFeedId = getEncodedRSSFeedId(decodedGroupId); - GroupRecord groupRecord = null; - if (!groupDatabase.isUnknownGroup(signalGroupId)) { - groupRecord = groupDatabase.getGroup(signalGroupId).orNull(); - } else if (!groupDatabase.isUnknownGroup(publicChatId)) { - groupRecord = groupDatabase.getGroup(publicChatId).orNull(); - } else if (!groupDatabase.isUnknownGroup(rssFeedId)) { - groupRecord = groupDatabase.getGroup(rssFeedId).orNull(); - } - return (groupRecord != null) ? groupRecord.getMembers() : null; - } } }