Add group admins and the ability to remove members from private groups

_locales/en/messages.json

@@ -309,6 +309,11 @@
     "description":
       "Displayed when a user can't send a message because they have left the group"
   },
+  "youGotKickedFromGroup": {
+    "message": "You were removed from the group",
+    "description":
+      "Displayed when a user can't send a message because they have left the group"
+  },
   "scrollDown": {
     "message": "Scroll to bottom of conversation",
     "description":
@@ -1839,6 +1844,28 @@
       }
     }
   },
+  "kickedFromTheGroup": {
+    "message": "$name$ was removed from the group",
+    "description":
+      "Shown in the conversation history when a single person is removed from the group",
+    "placeholders": {
+      "name": {
+        "content": "$1",
+        "example": "Alice"
+      }
+    }
+  },
+  "multipleKickedFromTheGroup": {
+    "message": "$names$ were removed from the group",
+    "description":
+      "Shown in the conversation history when more than one person is removed from the group",
+    "placeholders": {
+      "names": {
+        "content": "$1",
+        "example": "Alice, Bob"
+      }
+    }
+  },
   "friendRequestPending": {
     "message": "Friend request",
     "description":
@@ -2128,5 +2155,8 @@
   },
   "maxGroupMembersError": {
     "message": "Max number of members for small group chats is: "
+  },
+  "nonAdminDeleteMember": {
+    "message": "Only group admin can remove members!"
   }
 }

js/background.js

@@ -701,17 +701,25 @@
         },
       };
 
-      await onMessageReceived(ev);
+      const convo = await ConversationController.getOrCreateAndWait(
+        groupId,
+        'group'
+      );
 
       const avatar = '';
       const options = {};
-      textsecure.messaging.updateGroup(
+
+      const recipients = _.union(convo.get('members'), members);
+
+      await onMessageReceived(ev);
+      convo.updateGroup({
         groupId,
         groupName,
         avatar,
+        recipients,
         members,
-        options
-      );
+        options,
+      });
     };
 
     window.doCreateGroup = async (groupName, members) => {
@@ -722,10 +730,13 @@
 
       const ourKey = textsecure.storage.user.getNumber();
 
+      const allMembers = [ourKey, ...members];
+
       ev.groupDetails = {
         id: groupId,
         name: groupName,
-        members: [ourKey, ...members],
+        members: allMembers,
+        recipients: allMembers,
         active: true,
         expireTimer: 0,
         avatar: '',
@@ -748,6 +759,8 @@
         window.friends.friendRequestStatusEnum.friends
       );
 
+      convo.updateGroupAdmins([ourKey]);
+
       appView.openConversation(groupId, {});
     };
 
@@ -1005,7 +1018,6 @@
     messageReceiver.addEventListener('delivery', onDeliveryReceipt);
     messageReceiver.addEventListener('contact', onContactReceived);
     messageReceiver.addEventListener('group', onGroupReceived);
-    window.addEventListener('group', onGroupReceived);
     messageReceiver.addEventListener('sent', onSentMessage);
     messageReceiver.addEventListener('readSync', onReadSync);
     messageReceiver.addEventListener('read', onReadReceipt);

js/models/conversations.js

@@ -84,6 +84,8 @@
         unlockTimestamp: null, // Timestamp used for expiring friend requests.
         sessionResetStatus: SessionResetEnum.none,
         swarmNodes: [],
+        groupAdmins: [],
+        isKickedFromGroup: false,
         isOnline: false,
       };
     },
@@ -671,6 +673,10 @@
         this.trigger('disable:input', true);
         return;
       }
+      if (this.get('isKickedFromGroup')) {
+        this.trigger('disable:input', true);
+        return;
+      }
       if (!this.isPrivate() && this.get('left')) {
         this.trigger('disable:input', true);
         this.trigger('change:placeholder', 'left-group');
@@ -715,6 +721,12 @@
         this.updateTextInputState();
       }
     },
+    async updateGroupAdmins(groupAdmins) {
+      this.set({ groupAdmins });
+      await window.Signal.Data.updateConversation(this.id, this.attributes, {
+        Conversation: Whisper.Conversation,
+      });
+    },
     async respondToAllFriendRequests(options) {
       const { response, status, direction = null } = options;
       // Ignore if no response supplied
@@ -1933,6 +1945,7 @@
             this.get('name'),
             this.get('avatar'),
             this.get('members'),
+            groupUpdate.recipients,
             options
           )
         )
@@ -2710,6 +2723,23 @@
         return;
       }
 
+      // For groups, block typing messages from non-members (e.g. from kicked members)
+      if (this.get('type') === 'group') {
+        const knownMembers = this.get('members');
+
+        if (knownMembers) {
+          const fromMember = knownMembers.indexOf(sender) !== -1;
+
+          if (!fromMember) {
+            window.log.warn(
+              'Blocking typing messages from a non-member: ',
+              sender
+            );
+            return;
+          }
+        }
+      }
+
       const identifier = `${sender}.${senderDevice}`;
 
       this.contactTypingTimers = this.contactTypingTimers || {};

js/models/messages.js

@@ -204,8 +204,12 @@
           return i18n('leftTheGroup', this.getNameForNumber(groupUpdate.left));
         }
 
+        if (groupUpdate.kicked === 'You') {
+          return i18n('youGotKickedFromGroup');
+        }
+
         const messages = [];
-        if (!groupUpdate.name && !groupUpdate.joined) {
+        if (!groupUpdate.name && !groupUpdate.joined && !groupUpdate.kicked) {
           messages.push(i18n('updatedTheGroup'));
         }
         if (groupUpdate.name) {
@@ -223,6 +227,18 @@
           }
         }
 
+        if (groupUpdate.kicked && groupUpdate.kicked.length) {
+          const names = _.map(
+            groupUpdate.kicked,
+            this.getNameForNumber.bind(this)
+          );
+
+          if (names.length > 1) {
+            messages.push(i18n('multipleKickedFromTheGroup', names.join(', ')));
+          } else {
+            messages.push(i18n('kickedFromTheGroup', names[0]));
+          }
+        }
         return messages.join(', ');
       }
       if (this.isEndSession()) {
@@ -462,6 +478,23 @@
         });
       }
 
+      if (groupUpdate.kicked === 'You') {
+        changes.push({
+          type: 'kicked',
+          isMe: true,
+        });
+      } else if (groupUpdate.kicked) {
+        changes.push({
+          type: 'kicked',
+          contacts: _.map(
+            Array.isArray(groupUpdate.kicked)
+              ? groupUpdate.kicked
+              : [groupUpdate.kicked],
+            phoneNumber => this.findAndFormatContact(phoneNumber)
+          ),
+        });
+      }
+
       if (groupUpdate.left === 'You') {
         changes.push({
           type: 'remove',
@@ -1705,16 +1738,65 @@
 
       const conversation = ConversationController.get(conversationId);
 
+      // NOTE: we use friends status to tell if this is
+      // the creation of the group (initial update)
+      const newGroup = !conversation.isFriend();
+      const knownMembers = conversation.get('members');
+
+      if (!newGroup && knownMembers) {
+        const fromMember = knownMembers.indexOf(source) !== -1;
+
+        if (!fromMember) {
+          window.log.warn(`Ignoring group message from non-member: ${source}`);
+          confirm();
+          return null;
+        }
+      }
+
       if (
         initialMessage.group &&
         initialMessage.group.members &&
         initialMessage.group.type === GROUP_TYPES.UPDATE
       ) {
-        // Note: this might be called more times than necessary
-        conversation.setFriendRequestStatus(
-          window.friends.friendRequestStatusEnum.friends
-        );
+        if (newGroup) {
+          conversation.updateGroupAdmins(initialMessage.group.admins);
+
+          conversation.setFriendRequestStatus(
+            window.friends.friendRequestStatusEnum.friends
+          );
+        }
+
+        const fromAdmin =
+          conversation.get('groupAdmins').indexOf(source) !== -1;
+
+        if (!fromAdmin) {
+          // Make sure the message is not removing members / renaming the group
+          const nameChanged =
+            conversation.get('name') !== initialMessage.group.name;
+
+          if (nameChanged) {
+            window.log.warn(
+              'Non-admin attempts to change the name of the group'
+            );
+          }
+
+          const membersMissing =
+            _.difference(
+              conversation.get('members'),
+              initialMessage.group.members
+            ).length > 0;
+
+          if (membersMissing) {
+            window.log.warn('Non-admin attempts to remove group members');
+          }
 
+          const messageAllowed = !nameChanged && !membersMissing;
+
+          if (!messageAllowed) {
+            confirm();
+            return null;
+          }
+        }
         // For every member, see if we need to establish a session:
         initialMessage.group.members.forEach(memberPubKey => {
           const haveSession = _.some(
@@ -1797,10 +1879,7 @@
               attributes = {
                 ...attributes,
                 name: dataMessage.group.name,
-                members: _.union(
-                  dataMessage.group.members,
-                  conversation.get('members')
-                ),
+                members: dataMessage.group.members,
               };
 
               groupUpdate =
@@ -1808,12 +1887,12 @@
                   _.pick(dataMessage.group, 'name', 'avatar')
                 ) || {};
 
-              const difference = _.difference(
+              const addedMembers = _.difference(
                 attributes.members,
                 conversation.get('members')
               );
-              if (difference.length > 0) {
-                groupUpdate.joined = difference;
+              if (addedMembers.length > 0) {
+                groupUpdate.joined = addedMembers;
               }
               if (conversation.get('left')) {
                 // TODO: Maybe we shouldn't assume this message adds us:
@@ -1821,6 +1900,30 @@
                 window.log.warn('re-added to a left group');
                 attributes.left = false;
               }
+
+              if (attributes.isKickedFromGroup) {
+                // Assume somebody re-invited us since we received this update
+                attributes.isKickedFromGroup = false;
+              }
+
+              // Check if anyone got kicked:
+              const removedMembers = _.difference(
+                conversation.get('members'),
+                attributes.members
+              );
+
+              if (removedMembers.length > 0) {
+                if (
+                  removedMembers.indexOf(
+                    textsecure.storage.user.getNumber()
+                  ) !== -1
+                ) {
+                  groupUpdate.kicked = 'You';
+                  attributes.isKickedFromGroup = true;
+                } else {
+                  groupUpdate.kicked = removedMembers;
+                }
+              }
             } else if (dataMessage.group.type === GROUP_TYPES.QUIT) {
               if (source === textsecure.storage.user.getNumber()) {
                 attributes.left = true;

js/modules/data.js

@@ -2,6 +2,8 @@
 
 const electron = require('electron');
 
+// TODO: this results in poor readability, would be
+// much better to implicitly call with `_`.
 const {
   cloneDeep,
   forEach,
@@ -9,11 +11,12 @@ const {
   isFunction,
   isObject,
   map,
-  merge,
   set,
   omit,
 } = require('lodash');
 
+const _ = require('lodash');
+
 const { base64ToArrayBuffer, arrayBufferToBase64 } = require('./crypto');
 const MessageType = require('./types/message');
 
@@ -662,17 +665,6 @@ async function getAllSessions(id) {
 
 // Conversation
 
-function setifyProperty(data, propertyName) {
-  if (!data) {
-    return data;
-  }
-  const returnData = { ...data };
-  if (Array.isArray(returnData[propertyName])) {
-    returnData[propertyName] = new Set(returnData[propertyName]);
-  }
-  return returnData;
-}
-
 async function getSwarmNodesByPubkey(pubkey) {
   return channels.getSwarmNodesByPubkey(pubkey);
 }
@@ -701,13 +693,14 @@ async function updateConversation(id, data, { Conversation }) {
   if (!existing) {
     throw new Error(`Conversation ${id} does not exist!`);
   }
-  const setData = setifyProperty(data, 'swarmNodes');
-  const setExisting = setifyProperty(existing.attributes, 'swarmNodes');
 
-  const merged = merge({}, setExisting, setData);
-  if (merged.swarmNodes instanceof Set) {
-    merged.swarmNodes = Array.from(merged.swarmNodes);
-  }
+  const merged = _.merge({}, existing.attributes, data);
+
+  // Merging is a really bad idea and not what we want here, e.g.
+  // it will take a union of old and new members and that's not
+  // what we want for member deletion, so:
+  merged.members = data.members;
+  merged.swarmNodes = data.swarmNodes;
 
   // Don't save the online status of the object
   const cleaned = omit(merged, 'isOnline');

js/views/conversation_view.js

@@ -2548,11 +2548,12 @@
           .models.filter(d => d.isPrivate());
         const memberConvos = members
           .map(m => privateConvos.find(c => c.id === m))
-          .filter(c => !!c);
-        allMembers = memberConvos.map(m => ({
-          id: m.id,
-          authorPhoneNumber: m.id,
-          authorProfileName: m.getLokiProfile().displayName,
+          .filter(c => !!c && c.getLokiProfile());
+
+        allMembers = memberConvos.map(c => ({
+          id: c.id,
+          authorPhoneNumber: c.id,
+          authorProfileName: c.getLokiProfile().displayName,
         }));
       }
 

js/views/create_group_dialog_view.js

@@ -1,4 +1,4 @@
-/* global Whisper, i18n, getInboxCollection _ */
+/* global Whisper, i18n, textsecure, _ */
 
 // eslint-disable-next-line func-names
 (function() {
@@ -17,7 +17,9 @@
       const convos = window.getConversations().models;
 
       let allMembers = convos.filter(d => !!d);
-      allMembers = allMembers.filter(d => d.isFriend() && d.isPrivate());
+      allMembers = allMembers.filter(
+        d => d.isFriend() && d.isPrivate() && !d.isMe()
+      );
       allMembers = _.uniq(allMembers, true, d => d.id);
 
       this.membersToShow = allMembers;
@@ -99,18 +101,22 @@
       this.close = this.close.bind(this);
       this.onSubmit = this.onSubmit.bind(this);
 
-      const convos = getInboxCollection().models;
+      const ourPK = textsecure.storage.user.getNumber();
+
+      this.isAdmin = groupConvo.get('groupAdmins').indexOf(ourPK) !== -1;
+
+      const convos = window.getConversations().models;
 
       let allMembers = convos.filter(d => !!d);
-      allMembers = allMembers.filter(d => d.isFriend());
-      allMembers = allMembers.filter(d => d.isPrivate());
+      allMembers = allMembers.filter(
+        d => d.isFriend() && d.isPrivate() && !d.isMe()
+      );
       allMembers = _.uniq(allMembers, true, d => d.id);
 
+      this.friendList = allMembers;
+
       // only give members that are not already in the group
       const existingMembers = groupConvo.get('members');
-      this.membersToShow = allMembers.filter(
-        d => !_.some(existingMembers, x => x === d.id)
-      );
 
       this.existingMembers = existingMembers;
 
@@ -127,7 +133,8 @@
           okText: this.okText,
           cancelText: this.cancelText,
           existingMembers: this.existingMembers,
-          friendList: this.membersToShow,
+          friendList: this.friendList,
+          isAdmin: this.isAdmin,
           onClose: this.close,
           onSubmit: this.onSubmit,
         },
@@ -137,10 +144,8 @@
       return this;
     },
     onSubmit(newGroupName, newMembers) {
-      const allMembers = window.Lodash.concat(
-        newMembers,
-        this.conversation.get('members')
-      );
+      const ourPK = textsecure.storage.user.getNumber();
+      const allMembers = window.Lodash.concat(newMembers, [ourPK]);
       const groupId = this.conversation.get('id');
 
       window.doUpdateGroup(groupId, newGroupName, allMembers);

js/views/group_update_view.js

@@ -6,6 +6,7 @@
 
   window.Whisper = window.Whisper || {};
 
+  // TODO: remove this as unused?
   Whisper.GroupUpdateView = Backbone.View.extend({
     tagName: 'div',
     className: 'group-update',

libtextsecure/sendmessage.js

@@ -1022,25 +1022,25 @@ MessageSender.prototype = {
     return this.sendMessage(attrs, options);
   },
 
-  updateGroup(groupId, name, avatar, targetNumbers, options) {
+  updateGroup(groupId, name, avatar, members, recipients, options) {
     const proto = new textsecure.protobuf.DataMessage();
     proto.group = new textsecure.protobuf.GroupContext();
 
     proto.group.id = stringToArrayBuffer(groupId);
     proto.group.type = textsecure.protobuf.GroupContext.Type.UPDATE;
     proto.group.name = name;
-    proto.group.members = targetNumbers;
+    proto.group.members = members;
+
+    const ourPK = textsecure.storage.user.getNumber();
+    proto.group.admins = [ourPK];
 
     return this.makeAttachmentPointer(avatar).then(attachment => {
       proto.group.avatar = attachment;
       // TODO: re-enable this once we have attachments
       proto.group.avatar = null;
-      return this.sendGroupProto(
-        targetNumbers,
-        proto,
-        Date.now(),
-        options
-      ).then(() => proto.group.id);
+      return this.sendGroupProto(recipients, proto, Date.now(), options).then(
+        () => proto.group.id
+      );
     });
   },
 

protos/SignalService.proto

@@ -337,6 +337,7 @@ message GroupContext {
   optional string            name    = 3;
   repeated string            members = 4;
   optional AttachmentPointer avatar  = 5;
+  repeated string            admins  = 6;
 }
 
 message ContactDetails {

stylesheets/_mentions.scss

@@ -75,6 +75,7 @@
 .friend-selection-list {
   max-height: 240px;
   overflow-y: scroll;
+  margin: 4px;
 
   .check-mark {
     float: right;
@@ -87,6 +88,14 @@
   .invisible {
     visibility: hidden;
   }
+
+  .existing-member {
+    color: green;
+  }
+
+  .existing-member-kicked {
+    color: red;
+  }
 }
 
 .dark-theme {

ts/components/conversation/AddMentions.tsx

@@ -89,6 +89,13 @@ class Mention extends React.Component<MentionProps, MentionState> {
       return d.id === this.props.convoId;
     });
 
+    if (!thisConvo) {
+      // If this gets triggered, is is likely because we deleted the conversation
+      this.clearOurInterval();
+
+      return;
+    }
+
     if (thisConvo.isPublic()) {
       // TODO: make this work for other public chats as well
       groupMembers = window.lokiPublicChatAPI
@@ -106,10 +113,14 @@ class Mention extends React.Component<MentionProps, MentionState> {
         .map((m: any) => privateConvos.find((c: any) => c.id === m))
         .filter((c: any) => !!c);
       groupMembers = memberConversations.map((m: any) => {
+        const name = m.getLokiProfile()
+          ? m.getLokiProfile().displayName
+          : m.attributes.displayName;
+
         return {
           id: m.id,
           authorPhoneNumber: m.id,
-          authorProfileName: m.getLokiProfile().displayName,
+          authorProfileName: name,
         };
       });
     }

ts/components/conversation/GroupNotification.tsx

@@ -15,7 +15,7 @@ interface Contact {
 }
 
 interface Change {
-  type: 'add' | 'remove' | 'name' | 'general';
+  type: 'add' | 'remove' | 'name' | 'general' | 'kicked';
   isMe: boolean;
   newName?: string;
   contacts?: Array<Contact>;
@@ -78,6 +78,21 @@ export class GroupNotification extends React.Component<Props> {
           contacts.length > 1 ? 'multipleLeftTheGroup' : 'leftTheGroup';
 
         return <Intl i18n={i18n} id={leftKey} components={[people]} />;
+      case 'kicked':
+        if (isMe) {
+          return i18n('youGotKickedFromGroup');
+        }
+
+        if (!contacts || !contacts.length) {
+          throw new Error('Group update is missing contacts');
+        }
+
+        const kickedKey =
+          contacts.length > 1
+            ? 'multipleKickedFromTheGroup'
+            : 'kickedFromTheGroup';
+
+        return <Intl i18n={i18n} id={kickedKey} components={[people]} />;
       case 'general':
         return i18n('updatedTheGroup');
       default:

ts/components/conversation/MemberList.tsx

@@ -11,10 +11,12 @@ export interface Contact {
   authorColor: any;
   authorAvatarPath: string;
   checkmarked: boolean;
+  existingMember: boolean;
 }
 interface MemberItemProps {
   member: Contact;
   selected: boolean;
+  existingMember: boolean;
   onClicked: any;
   i18n: any;
   checkmarked: boolean;
@@ -30,10 +32,41 @@ class MemberItem extends React.Component<MemberItemProps> {
     const name = this.props.member.authorProfileName;
     const pubkey = this.props.member.authorPhoneNumber;
     const selected = this.props.selected;
+    const existingMember = this.props.existingMember;
 
-    const checkMarkClass = this.props.checkmarked
-      ? 'check-mark'
-      : classNames('check-mark', 'invisible');
+    let markType: 'none' | 'kicked' | 'added' | 'existing' = 'none';
+
+    if (this.props.checkmarked) {
+      if (existingMember) {
+        markType = 'kicked';
+      } else {
+        markType = 'added';
+      }
+    } else {
+      if (existingMember) {
+        markType = 'existing';
+      } else {
+        markType = 'none';
+      }
+    }
+
+    const markClasses = ['check-mark'];
+
+    switch (markType) {
+      case 'none':
+        markClasses.push('invisible');
+        break;
+      case 'existing':
+        markClasses.push('existing-member');
+        break;
+      case 'kicked':
+        markClasses.push('existing-member-kicked');
+        break;
+      default:
+      // do nothing
+    }
+
+    const mark = markType === 'kicked' ? '✘' : '✔';
 
     return (
       <div
@@ -47,7 +80,7 @@ class MemberItem extends React.Component<MemberItemProps> {
         {this.renderAvatar()}
         <span className="name-part">{name}</span>
         <span className="pubkey-part">{pubkey}</span>
-        <span className={checkMarkClass}>✓</span>
+        <span className={classNames(markClasses)}>{mark}</span>
       </div>
     );
   }
@@ -98,6 +131,7 @@ export class MemberList extends React.Component<MemberListProps> {
           member={item}
           selected={selected}
           checkmarked={item.checkmarked}
+          existingMember={item.existingMember}
           i18n={this.props.i18n}
           onClicked={this.handleMemberClicked}
         />

ts/components/conversation/UpdateGroupDialog.tsx

@@ -5,6 +5,7 @@ import { Contact, MemberList } from './MemberList';
 declare global {
   interface Window {
     SMALL_GROUP_SIZE_LIMIT: number;
+    Lodash: any;
   }
 }
 
@@ -15,6 +16,7 @@ interface Props {
   cancelText: string;
   // friends not in the group
   friendList: Array<any>;
+  isAdmin: boolean;
   existingMembers: Array<any>;
   i18n: any;
   onSubmit: any;
@@ -43,6 +45,8 @@ export class UpdateGroupDialog extends React.Component<Props, State> {
       const lokiProfile = d.getLokiProfile();
       const name = lokiProfile ? lokiProfile.displayName : 'Anonymous';
 
+      const existingMember = this.props.existingMembers.indexOf(d.id) !== -1;
+
       return {
         id: d.id,
         authorPhoneNumber: d.id,
@@ -51,6 +55,7 @@ export class UpdateGroupDialog extends React.Component<Props, State> {
         authorName: name, // different from ProfileName?
         authorColor: d.getColor(),
         checkmarked: false,
+        existingMember,
       };
     });
 
@@ -65,9 +70,9 @@ export class UpdateGroupDialog extends React.Component<Props, State> {
   }
 
   public onClickOK() {
-    const members = this.state.friendList
-      .filter(d => d.checkmarked)
-      .map(d => d.id);
+    const members = this.getWouldBeMembers(this.state.friendList).map(
+      d => d.id
+    );
 
     if (!this.state.groupName.trim()) {
       this.onShowError(this.props.i18n('emptyGroupNameError'));
@@ -81,7 +86,7 @@ export class UpdateGroupDialog extends React.Component<Props, State> {
   }
 
   public render() {
-    const checkMarkedCount = this.getMemberCount();
+    const checkMarkedCount = this.getMemberCount(this.state.friendList);
 
     const titleText = `${this.props.titleText} (Members: ${checkMarkedCount})`;
 
@@ -109,6 +114,7 @@ export class UpdateGroupDialog extends React.Component<Props, State> {
           className="group-name"
           placeholder="Group Name"
           value={this.state.groupName}
+          disabled={!this.props.isAdmin}
           onChange={this.onGroupNameChanged}
           tabIndex={0}
           required={true}
@@ -166,11 +172,20 @@ export class UpdateGroupDialog extends React.Component<Props, State> {
     }
   }
 
-  private getMemberCount() {
-    return (
-      this.props.existingMembers.length +
-      this.state.friendList.filter(d => d.checkmarked).length
-    );
+  // Return members that would comprise the group given the
+  // current state in `users`
+  private getWouldBeMembers(users: Array<Contact>) {
+    return users.filter(d => {
+      return (
+        (d.existingMember && !d.checkmarked) ||
+        (!d.existingMember && d.checkmarked)
+      );
+    });
+  }
+
+  private getMemberCount(users: Array<Contact>) {
+    // Adding one to include ourselves
+    return this.getWouldBeMembers(users).length + 1;
   }
 
   private closeDialog() {
@@ -180,6 +195,12 @@ export class UpdateGroupDialog extends React.Component<Props, State> {
   }
 
   private onMemberClicked(selected: any) {
+    if (selected.existingMember && !this.props.isAdmin) {
+      this.onShowError(this.props.i18n('nonAdminDeleteMember'));
+
+      return;
+    }
+
     const updatedFriends = this.state.friendList.map(member => {
       if (member.id === selected.id) {
         return { ...member, checkmarked: !member.checkmarked };
@@ -188,11 +209,9 @@ export class UpdateGroupDialog extends React.Component<Props, State> {
       }
     });
 
-    const newMemberCount =
-      this.props.existingMembers.length +
-      updatedFriends.filter(d => d.checkmarked).length;
+    const newMemberCount = this.getMemberCount(updatedFriends);
 
-    if (newMemberCount > window.SMALL_GROUP_SIZE_LIMIT - 1) {
+    if (newMemberCount > window.SMALL_GROUP_SIZE_LIMIT) {
       const msg = `${this.props.i18n('maxGroupMembersError')} ${
         window.SMALL_GROUP_SIZE_LIMIT
       }`;