From 6c6eed0b1e1e95aeff120268289e5f071d727137 Mon Sep 17 00:00:00 2001
From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com>
Date: Wed, 2 Jun 2021 12:24:22 -0500
Subject: [PATCH] Don't show name collisions for system contacts

---
 ts/components/CallingParticipantsList.tsx     |  3 +-
 ts/components/ContactListItem.tsx             | 15 ++++--
 ts/components/SafetyNumberChangeDialog.tsx    |  3 +-
 .../ContactSpoofingReviewDialog.tsx           |  3 +-
 .../conversation/ConversationHeader.tsx       |  5 +-
 ts/state/selectors/conversations.ts           |  5 +-
 .../util/groupMemberNameCollisions_test.ts    | 29 ++++++++++++
 ts/test-both/util/isInSystemContacts_test.ts  | 46 +++++++++++++++++++
 ts/util/getSendOptions.ts                     |  7 ++-
 ts/util/groupMemberNameCollisions.ts          |  4 +-
 ts/util/isConversationAccepted.ts             |  6 ++-
 ts/util/isInSystemContacts.ts                 | 12 +++++
 12 files changed, 121 insertions(+), 17 deletions(-)
 create mode 100644 ts/test-both/util/isInSystemContacts_test.ts
 create mode 100644 ts/util/isInSystemContacts.ts

diff --git a/ts/components/CallingParticipantsList.tsx b/ts/components/CallingParticipantsList.tsx
index f6f679fde..b3a124573 100644
--- a/ts/components/CallingParticipantsList.tsx
+++ b/ts/components/CallingParticipantsList.tsx
@@ -11,6 +11,7 @@ import { InContactsIcon } from './InContactsIcon';
 import { LocalizerType } from '../types/Util';
 import { sortByTitle } from '../util/sortByTitle';
 import { ConversationType } from '../state/ducks/conversations';
+import { isInSystemContacts } from '../util/isInSystemContacts';
 
 type ParticipantType = ConversationType & {
   hasRemoteAudio?: boolean;
@@ -118,7 +119,7 @@ export const CallingParticipantsList = React.memo(
                           module="module-calling-participants-list__name"
                           title={participant.title}
                         />
-                        {participant.name ? (
+                        {isInSystemContacts(participant) ? (
                           <span>
                             {' '}
                             <InContactsIcon
diff --git a/ts/components/ContactListItem.tsx b/ts/components/ContactListItem.tsx
index b67d12c7b..932544eb2 100644
--- a/ts/components/ContactListItem.tsx
+++ b/ts/components/ContactListItem.tsx
@@ -11,6 +11,7 @@ import { InContactsIcon } from './InContactsIcon';
 
 import { LocalizerType } from '../types/Util';
 import { ConversationType } from '../state/ducks/conversations';
+import { isInSystemContacts } from '../util/isInSystemContacts';
 
 type Props = {
   i18n: LocalizerType;
@@ -69,10 +70,18 @@ export class ContactListItem extends React.Component<Props> {
   }
 
   public render(): JSX.Element {
-    const { about, i18n, isAdmin, isMe, name, onClick, title } = this.props;
+    const {
+      about,
+      i18n,
+      isAdmin,
+      isMe,
+      name,
+      onClick,
+      title,
+      type,
+    } = this.props;
 
     const displayName = isMe ? i18n('you') : title;
-    const shouldShowIcon = Boolean(name);
 
     return (
       <button
@@ -88,7 +97,7 @@ export class ContactListItem extends React.Component<Props> {
           <div className="module-contact-list-item__left">
             <div className="module-contact-list-item__text__name">
               <Emojify text={displayName} />
-              {shouldShowIcon ? (
+              {isInSystemContacts({ name, type }) ? (
                 <span>
                   {' '}
                   <InContactsIcon i18n={i18n} />
diff --git a/ts/components/SafetyNumberChangeDialog.tsx b/ts/components/SafetyNumberChangeDialog.tsx
index 5e5926dc5..70fec246c 100644
--- a/ts/components/SafetyNumberChangeDialog.tsx
+++ b/ts/components/SafetyNumberChangeDialog.tsx
@@ -11,6 +11,7 @@ import { Modal } from './Modal';
 
 import { ConversationType } from '../state/ducks/conversations';
 import { LocalizerType } from '../types/Util';
+import { isInSystemContacts } from '../util/isInSystemContacts';
 
 export type SafetyNumberProps = {
   contactID: string;
@@ -103,7 +104,7 @@ export const SafetyNumberChangeDialog = ({
               <div className="module-SafetyNumberChangeDialog__contact--wrapper">
                 <div className="module-SafetyNumberChangeDialog__contact--name">
                   {contact.title}
-                  {contact.name ? (
+                  {isInSystemContacts(contact) ? (
                     <span>
                       {' '}
                       <InContactsIcon i18n={i18n} />
diff --git a/ts/components/conversation/ContactSpoofingReviewDialog.tsx b/ts/components/conversation/ContactSpoofingReviewDialog.tsx
index 103778212..6ad12fd82 100644
--- a/ts/components/conversation/ContactSpoofingReviewDialog.tsx
+++ b/ts/components/conversation/ContactSpoofingReviewDialog.tsx
@@ -25,6 +25,7 @@ import { Intl } from '../Intl';
 import { Emojify } from './Emojify';
 import { assert } from '../../util/assert';
 import { missingCaseError } from '../../util/missingCaseError';
+import { isInSystemContacts } from '../../util/isInSystemContacts';
 
 type PropsType = {
   i18n: LocalizerType;
@@ -271,7 +272,7 @@ export const ContactSpoofingReviewDialog: FunctionComponent<PropsType> = props =
                   {i18n('MessageRequests--unblock')}
                 </Button>
               );
-            } else if (!conversationInfo.conversation.name) {
+            } else if (!isInSystemContacts(conversationInfo.conversation)) {
               button = (
                 <Button
                   variant={ButtonVariant.SecondaryDestructive}
diff --git a/ts/components/conversation/ConversationHeader.tsx b/ts/components/conversation/ConversationHeader.tsx
index f3b3e352d..73e65beb9 100644
--- a/ts/components/conversation/ConversationHeader.tsx
+++ b/ts/components/conversation/ConversationHeader.tsx
@@ -23,6 +23,7 @@ import { MuteOption, getMuteOptions } from '../../util/getMuteOptions';
 import * as expirationTimer from '../../util/expirationTimer';
 import { isMuted } from '../../util/isMuted';
 import { missingCaseError } from '../../util/missingCaseError';
+import { isInSystemContacts } from '../../util/isInSystemContacts';
 
 export enum OutgoingCallButtonStyle {
   None,
@@ -155,12 +156,10 @@ export class ConversationHeader extends React.Component<PropsType, StateType> {
       );
     }
 
-    const shouldShowIcon = Boolean(name && type === 'direct');
-
     return (
       <div className="module-ConversationHeader__header__info__title">
         <Emojify text={title} />
-        {shouldShowIcon ? (
+        {isInSystemContacts({ name, type }) ? (
           <InContactsIcon
             className="module-ConversationHeader__header__info__title__in-contacts-icon"
             i18n={i18n}
diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts
index b8145b4ef..4d6b573d6 100644
--- a/ts/state/selectors/conversations.ts
+++ b/ts/state/selectors/conversations.ts
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import memoizee from 'memoizee';
-import { fromPairs, isNumber, isString } from 'lodash';
+import { fromPairs, isNumber } from 'lodash';
 import { createSelector } from 'reselect';
 
 import { StateType } from '../reducer';
@@ -38,6 +38,7 @@ import {
   getUserNumber,
 } from './user';
 import { getPinnedConversationIds } from './items';
+import { isInSystemContacts } from '../../util/isInSystemContacts';
 
 let placeholderContact: ConversationType;
 export const getPlaceholderContact = (): ConversationType => {
@@ -366,7 +367,7 @@ function isTrusted(conversation: ConversationType): boolean {
   }
 
   return Boolean(
-    isString(conversation.name) ||
+    isInSystemContacts(conversation) ||
       conversation.profileSharing ||
       conversation.isMe
   );
diff --git a/ts/test-both/util/groupMemberNameCollisions_test.ts b/ts/test-both/util/groupMemberNameCollisions_test.ts
index f3a574e09..e934876f0 100644
--- a/ts/test-both/util/groupMemberNameCollisions_test.ts
+++ b/ts/test-both/util/groupMemberNameCollisions_test.ts
@@ -46,6 +46,7 @@ describe('group member name collision utilities', () => {
         title: 'Bob',
       });
       const bob2 = getDefaultConversation({
+        name: 'Bob In Your Contacts',
         profileName: 'Bob',
         title: 'Bob',
       });
@@ -53,19 +54,44 @@ describe('group member name collision utilities', () => {
         profileName: 'Bob',
         title: 'Bob',
       });
+
+      // Ignored, because Bob is not in your contacts (lacks `name`), has no profile name,
+      //   and has no E164.
       const ignoredBob = getDefaultConversation({
         e164: undefined,
         title: 'Bob',
       });
+
+      // Ignored, because there's only one Charlie.
       const charlie = getDefaultConversation({
         profileName: 'Charlie',
         title: 'Charlie',
       });
+
+      // Ignored, because all Donnas are in your contacts (they have a `name`).
+      const donna1 = getDefaultConversation({
+        name: 'Donna One',
+        profileName: 'Donna',
+        title: 'Donna',
+      });
+      const donna2 = getDefaultConversation({
+        name: 'Donna Two',
+        profileName: 'Donna',
+        title: 'Donna',
+      });
+      const donna3 = getDefaultConversation({
+        name: 'Donna Three',
+        profileName: 'Donna',
+        title: 'Donna',
+      });
+
+      // Ignored, because you're not included.
       const me = getDefaultConversation({
         isMe: true,
         profileName: 'Alice',
         title: 'Alice',
       });
+
       const memberships = [
         alice1,
         alice2,
@@ -74,6 +100,9 @@ describe('group member name collision utilities', () => {
         bob3,
         ignoredBob,
         charlie,
+        donna1,
+        donna2,
+        donna3,
         me,
       ].map(member => ({ member }));
 
diff --git a/ts/test-both/util/isInSystemContacts_test.ts b/ts/test-both/util/isInSystemContacts_test.ts
new file mode 100644
index 000000000..70ee5623e
--- /dev/null
+++ b/ts/test-both/util/isInSystemContacts_test.ts
@@ -0,0 +1,46 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+
+import { isInSystemContacts } from '../../util/isInSystemContacts';
+
+describe('isInSystemContacts', () => {
+  it('returns true for direct conversations that have a `name` property', () => {
+    assert.isTrue(
+      isInSystemContacts({
+        type: 'direct',
+        name: 'Jane Doe',
+      })
+    );
+    assert.isTrue(
+      isInSystemContacts({
+        type: 'private',
+        name: 'Jane Doe',
+      })
+    );
+  });
+
+  it('returns true for direct conversations that have an empty string `name`', () => {
+    assert.isTrue(
+      isInSystemContacts({
+        type: 'direct',
+        name: '',
+      })
+    );
+  });
+
+  it('returns false for direct conversations that lack a `name` property', () => {
+    assert.isFalse(isInSystemContacts({ type: 'direct' }));
+  });
+
+  it('returns false for group conversations', () => {
+    assert.isFalse(isInSystemContacts({ type: 'group' }));
+    assert.isFalse(
+      isInSystemContacts({
+        type: 'group',
+        name: 'Tahoe Trip',
+      })
+    );
+  });
+});
diff --git a/ts/util/getSendOptions.ts b/ts/util/getSendOptions.ts
index e23d8da04..f6a4dc565 100644
--- a/ts/util/getSendOptions.ts
+++ b/ts/util/getSendOptions.ts
@@ -6,6 +6,7 @@ import { SendMetadataType, SendOptionsType } from '../textsecure/SendMessage';
 import { arrayBufferToBase64, getRandomBytes } from '../Crypto';
 import { getConversationMembers } from './getConversationMembers';
 import { isDirectConversation, isMe } from './whatTypeOfConversation';
+import { isInSystemContacts } from './isInSystemContacts';
 import { missingCaseError } from './missingCaseError';
 import { senderCertificateService } from '../services/senderCertificate';
 import {
@@ -117,13 +118,11 @@ function getSenderCertificateForDirectConversation(
     case PhoneNumberSharingMode.Everybody:
       certificateMode = SenderCertificateMode.WithE164;
       break;
-    case PhoneNumberSharingMode.ContactsOnly: {
-      const isInSystemContacts = Boolean(conversationAttrs.name);
-      certificateMode = isInSystemContacts
+    case PhoneNumberSharingMode.ContactsOnly:
+      certificateMode = isInSystemContacts(conversationAttrs)
         ? SenderCertificateMode.WithE164
         : SenderCertificateMode.WithoutE164;
       break;
-    }
     case PhoneNumberSharingMode.Nobody:
       certificateMode = SenderCertificateMode.WithoutE164;
       break;
diff --git a/ts/util/groupMemberNameCollisions.ts b/ts/util/groupMemberNameCollisions.ts
index a46fbae9f..35303f563 100644
--- a/ts/util/groupMemberNameCollisions.ts
+++ b/ts/util/groupMemberNameCollisions.ts
@@ -6,6 +6,7 @@ import { groupBy, map, filter } from './iterables';
 import { getOwn } from './getOwn';
 import { ConversationType } from '../state/ducks/conversations';
 import { isConversationNameKnown } from './isConversationNameKnown';
+import { isInSystemContacts } from './isInSystemContacts';
 
 export type GroupNameCollisionsWithIdsByTitle = Record<string, Array<string>>;
 export type GroupNameCollisionsWithConversationsByTitle = Record<
@@ -37,7 +38,8 @@ export function getCollisionsFromMemberships(
   // [0]: https://www.typescriptlang.org/play?#code/C4TwDgpgBAYg9nKBeKAFAhgJ2AS3QGwB4AlCAYzkwBNCBnYTHAOwHMAaKJgVwFsAjCJgB8QgNwAoCk3pQAZgC5YCZFADeUABY5FAVigBfCeNCQoAISwrSFanQbN2nXgOESpMvoouYVs0UA
   return (pickBy(
     groupedByTitle,
-    group => group.length >= 2
+    group =>
+      group.length >= 2 && !group.every(person => isInSystemContacts(person))
   ) as unknown) as GroupNameCollisionsWithConversationsByTitle;
 }
 
diff --git a/ts/util/isConversationAccepted.ts b/ts/util/isConversationAccepted.ts
index 268f1dfc5..07a34f289 100644
--- a/ts/util/isConversationAccepted.ts
+++ b/ts/util/isConversationAccepted.ts
@@ -3,6 +3,7 @@
 
 import { ConversationAttributesType } from '../model-types.d';
 import { isDirectConversation, isMe } from './whatTypeOfConversation';
+import { isInSystemContacts } from './isInSystemContacts';
 
 /**
  * Determine if this conversation should be considered "accepted" in terms
@@ -63,7 +64,10 @@ function isFromOrAddedByTrustedContact(
   conversationAttrs: ConversationAttributesType
 ): boolean {
   if (isDirectConversation(conversationAttrs)) {
-    return Boolean(conversationAttrs.name || conversationAttrs.profileSharing);
+    return (
+      isInSystemContacts(conversationAttrs) ||
+      Boolean(conversationAttrs.profileSharing)
+    );
   }
 
   const { addedBy } = conversationAttrs;
diff --git a/ts/util/isInSystemContacts.ts b/ts/util/isInSystemContacts.ts
new file mode 100644
index 000000000..35a5b7932
--- /dev/null
+++ b/ts/util/isInSystemContacts.ts
@@ -0,0 +1,12 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+export const isInSystemContacts = ({
+  type,
+  name,
+}: Readonly<{
+  type?: string;
+  name?: string;
+}>): boolean =>
+  // `direct` for redux, `private` for models and the database
+  (type === 'direct' || type === 'private') && typeof name === 'string';