Merge pull request #167 from loki-project/closed-group-fixes

Closed Group Fixes

src/org/thoughtcrime/securesms/conversation/ConversationActivity.java

@@ -317,7 +317,6 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
   private   Button                      makeDefaultSmsButton;
   private   Button                      registerButton;
   private   InputAwareLayout            container;
-  private   View                        composePanel;
   protected Stub<ReminderView>          reminderView;
   private   Stub<UnverifiedBannerView>  unverifiedBannerView;
   private   Stub<GroupShareProfileView> groupShareProfileView;
@@ -552,7 +551,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
     updateTitleTextView(recipient);
     updateSubtitleTextView();
     setActionBarColor(recipient.getColor());
-    setBlockedUserState(recipient, isSecureText, isDefaultSms);
+    updateInputUI(recipient, isSecureText, isDefaultSms);
     setGroupShareProfileReminder(recipient);
     calculateCharactersRemaining();
 
@@ -645,7 +644,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
       updateTitleTextView(recipient);
       updateSubtitleTextView();
       NotificationChannels.updateContactChannelName(this, recipient);
-      setBlockedUserState(recipient, isSecureText, isDefaultSms);
+      updateInputUI(recipient, isSecureText, isDefaultSms);
       supportInvalidateOptionsMenu();
       break;
     case TAKE_PHOTO:
@@ -858,6 +857,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
         searchViewModel.onSearchClosed();
         searchNav.setVisibility(View.GONE);
         inputPanel.setVisibility(View.VISIBLE);
+        updateInputUI(recipient, isSecureText, isDefaultSms);
         fragment.onSearchQueryUpdated(null);
         invalidateOptionsMenu();
         return true;
@@ -1343,7 +1343,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
 
     calculateCharactersRemaining();
     supportInvalidateOptionsMenu();
-    setBlockedUserState(recipient, isSecureText, isDefaultSms);
+    updateInputUI(recipient, isSecureText, isDefaultSms);
   }
 
   ///// Initializers
@@ -1627,7 +1627,6 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
     unblockButton          = ViewUtil.findById(this, R.id.unblock_button);
     makeDefaultSmsButton   = ViewUtil.findById(this, R.id.make_default_sms_button);
     registerButton         = ViewUtil.findById(this, R.id.register_button);
-    composePanel           = ViewUtil.findById(this, R.id.bottom_panel);
     container              = ViewUtil.findById(this, R.id.layout_container);
     reminderView           = ViewUtil.findStubById(this, R.id.reminder_stub);
     unverifiedBannerView   = ViewUtil.findStubById(this, R.id.unverified_banner_stub);
@@ -1835,7 +1834,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
       updateTitleTextView(recipient);
       updateSubtitleTextView();
 //      titleView.setVerified(identityRecords.isVerified());
-      setBlockedUserState(recipient, isSecureText, isDefaultSms);
+      updateInputUI(recipient, isSecureText, isDefaultSms);
       setActionBarColor(recipient.getColor());
       setGroupShareProfileReminder(recipient);
       updateReminders(recipient.hasSeenInviteReminder());
@@ -2043,29 +2042,30 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
     setStatusBarColor(getResources().getColor(R.color.action_bar_background));
   }
 
-  private void setBlockedUserState(Recipient recipient, boolean isSecureText, boolean isDefaultSms) {
+  // FIXME: This name is confusing because we also have updateInputPanel and setInputPanelEnabled
-    if (recipient.isGroupRecipient() && recipient.getAddress().isRSSFeed()) {
+  private void updateInputUI(Recipient recipient, boolean isSecureText, boolean isDefaultSms) {
+    if (recipient.isGroupRecipient() && !isActiveGroup()) {
       unblockButton.setVisibility(View.GONE);
-      composePanel.setVisibility(View.GONE);
+      inputPanel.setVisibility(View.GONE);
       makeDefaultSmsButton.setVisibility(View.GONE);
       registerButton.setVisibility(View.GONE);
     } else if (recipient.isBlocked()) {
       unblockButton.setVisibility(View.VISIBLE);
-      composePanel.setVisibility(View.GONE);
+      inputPanel.setVisibility(View.GONE);
       makeDefaultSmsButton.setVisibility(View.GONE);
       registerButton.setVisibility(View.GONE);
     } else if (!isSecureText && isPushGroupConversation()) {
       unblockButton.setVisibility(View.GONE);
-      composePanel.setVisibility(View.GONE);
+      inputPanel.setVisibility(View.GONE);
       makeDefaultSmsButton.setVisibility(View.GONE);
       registerButton.setVisibility(View.VISIBLE);
     } else if (!isSecureText && !isDefaultSms) {
       unblockButton.setVisibility(View.GONE);
-      composePanel.setVisibility(View.GONE);
+      inputPanel.setVisibility(View.GONE);
       makeDefaultSmsButton.setVisibility(View.VISIBLE);
       registerButton.setVisibility(View.GONE);
     } else {
-      composePanel.setVisibility(View.VISIBLE);
+      inputPanel.setVisibility(View.VISIBLE);
       unblockButton.setVisibility(View.GONE);
       makeDefaultSmsButton.setVisibility(View.GONE);
       registerButton.setVisibility(View.GONE);
@@ -2125,7 +2125,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
   }
 
   private boolean isActiveGroup() {
-    if (!isGroupConversation()) return false;
+    if (!isGroupConversation() || recipient.getAddress().isRSSFeed()) return false;
 
     Optional<GroupRecord> record = DatabaseFactory.getGroupDatabase(this).getGroup(getRecipient().getAddress().toGroupString());
     return record.isPresent() && record.get().isActive();
@@ -2314,7 +2314,7 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
       String hint = enabled ? "Message" : "Pending session request";
       inputPanel.setHint(hint);
       inputPanel.setEnabled(enabled);
-      if (enabled) {
+      if (enabled && inputPanel.getVisibility() == View.VISIBLE) {
         inputPanel.composeText.requestFocus();
         InputMethodManager inputMethodManager = (InputMethodManager) getSystemService(INPUT_METHOD_SERVICE);
         inputMethodManager.showSoftInput(inputPanel.composeText, 0);
@@ -2939,6 +2939,8 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
 
   @Override
   public void handleReplyMessage(MessageRecord messageRecord) {
+    if (recipient.isGroupRecipient() && !isActiveGroup()) { return; }
+
     Recipient author;
 
     if (messageRecord.isOutgoing()) {

src/org/thoughtcrime/securesms/groups/GroupMessageProcessor.java

@@ -137,49 +137,57 @@ public class GroupMessageProcessor {
     GroupDatabase database = DatabaseFactory.getGroupDatabase(context);
     String        id       = GroupUtil.getEncodedId(group);
 
-    // Only update group if admin sent the message
+    String ourHexEncodedPublicKey = getMasterHexEncodedPublicKey(context, TextSecurePreferences.getLocalNumber(context));
+
     if (group.getGroupType() == SignalServiceGroup.GroupType.SIGNAL) {
+      // Only update group if the group 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 messages if we're 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 a member of:  " + id + "; ignoring.");
+        database.setActive(id, false);
+        return null;
+      }
     }
 
-    Set<Address> recordMembers = new HashSet<>(groupRecord.getMembers());
+    Set<Address> currentMembers = new HashSet<>(groupRecord.getMembers());
-    Set<Address> messageMembers = new HashSet<>();
+    Set<Address> newMembers = new HashSet<>();
 
     for (String messageMember : group.getMembers().get()) {
-      messageMembers.add(Address.fromExternal(context, messageMember));
+      newMembers.add(Address.fromExternal(context, messageMember));
     }
 
-    Set<Address> addedMembers = new HashSet<>(messageMembers);
+    // Added members are the members who are present in newMembers but not in currentMembers
-    addedMembers.removeAll(recordMembers);
+    Set<Address> addedMembers = new HashSet<>(newMembers);
+    addedMembers.removeAll(currentMembers);
 
-    Set<Address> missingMembers = new HashSet<>(recordMembers);
+    // Kicked members are members who are present in currentMembers but not in newMembers
-    missingMembers.removeAll(messageMembers);
+    Set<Address> removedMembers = new HashSet<>(currentMembers);
+    removedMembers.removeAll(newMembers);
 
     GroupContext.Builder builder = createGroupContext(group);
     builder.setType(GroupContext.Type.UPDATE);
 
-    if (addedMembers.size() > 0) {
+    // Update our group members if they're different
-      Set<Address> unionMembers = new HashSet<>(recordMembers);
+    if (!currentMembers.equals(newMembers)) {
-      unionMembers.addAll(messageMembers);
+      database.updateMembers(id, new LinkedList<>(newMembers));
-      database.updateMembers(id, new LinkedList<>(unionMembers));
+    }
-
-      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 purposes
     for (Address addedMember : addedMembers) {
-        builder.addMembers(addedMember.serialize());
+      builder.addNewMembers(addedMember.serialize());
-      }
-    } else {
-      builder.clearMembers();
     }
 
-    if (missingMembers.size() > 0) {
+    for (Address removedMember : removedMembers) {
-      for (Address removedMember : missingMembers) {
+      builder.addRemovedMembers(removedMember.serialize());
-        builder.addMembers(removedMember.serialize());
-      }
     }
 
     if (group.getName().isPresent() || group.getAvatar().isPresent()) {
@@ -191,11 +199,16 @@ public class GroupMessageProcessor {
       builder.clearName();
     }
 
+    // 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());
       }
+    }
 
     return storeMessage(context, content, group, builder.build(), outgoing);
   }

src/org/thoughtcrime/securesms/jobs/PushDecryptJob.java

@@ -1895,7 +1895,28 @@ public class PushDecryptJob extends BaseJob implements InjectableType {
         boolean isGroupActive    = groupId.isPresent() && groupDatabase.isActive(groupId.get());
         boolean isLeaveMessage   = message.getGroupInfo().isPresent() && message.getGroupInfo().get().getType() == SignalServiceGroup.Type.QUIT;
 
-        return (isContentMessage && !isGroupActive) || (sender.isBlocked() && !isLeaveMessage);
+        boolean isClosedGroup = conversation.getAddress().isSignalGroup();
+        boolean isGroupMember = true;
+
+        // Only allow messages from group members
+        if (isClosedGroup) {
+          String senderHexEncodedPublicKey = content.getSender();
+
+          try {
+            String masterHexEncodedPublicKey = PromiseUtil.timeout(LokiDeviceLinkUtilities.INSTANCE.getMasterHexEncodedPublicKey(content.getSender()), 5000).get();
+            if (masterHexEncodedPublicKey != null) {
+              senderHexEncodedPublicKey = masterHexEncodedPublicKey;
+            }
+          } catch (Exception e) {
+            e.printStackTrace();
+          }
+
+          Recipient senderMasterAddress = Recipient.from(context, Address.fromSerialized(senderHexEncodedPublicKey), false);
+
+          isGroupMember = groupId.isPresent() && groupDatabase.getGroupMembers(groupId.get(), true).contains(senderMasterAddress);
+        }
+
+        return (isContentMessage && !isGroupActive) || (sender.isBlocked() && !isLeaveMessage) || (isContentMessage && !isGroupMember);
       } else {
         return sender.isBlocked();
       }

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<Recipient> members;
+    private final List<Recipient> newMembers;
     private final List<Recipient> 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()) {
+      if (groupContext != null) {
-        List<String> memberList = groupContext.getMembersList();
+        List<String> newMembers = groupContext.getNewMembersList();
-        List<Address> currentMembers = getCurrentGroupMembers();
+        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<String> 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(hexEncodedPublicKey);
-          ourDeviceWasRemoved = removedMembers.contains(self);
         }
       }
     }
 
+    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(" ");
+        String separator = (!newMembers.isEmpty() || !removedMembers.isEmpty()) ? " " : "\n";
-        else                 description.append("\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()) {
+      if (!this.newMembers.isEmpty()) {
-        for (Recipient member : this.members) {
+        for (Recipient member : this.newMembers) {
           member.addListener(listener);
         }
       }
@@ -252,23 +251,5 @@ public class GroupUtil {
 
     return result;
     }
-
-    private List<Address> 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;
-    }
   }
 }