Fix group send endorsements for new members

Co-authored-by: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com>

ts/groups.ts

@@ -3986,7 +3986,7 @@ async function updateGroupViaLogs({
 
   if (
     cachedEndorsementsExpiration != null &&
-    !isValidGroupSendEndorsementsExpiration(cachedEndorsementsExpiration)
+    !isValidGroupSendEndorsementsExpiration(cachedEndorsementsExpiration * 1000)
   ) {
     log.info(
       `updateGroupViaLogs/${logId}: Group had invalid endorsements expiration (${cachedEndorsementsExpiration}), fetching new endorsements`

ts/test-mock/playwright.ts

@@ -144,8 +144,24 @@ export class App extends EventEmitter {
     );
   }
 
+  private async checkForFatalTestErrors(): Promise<void> {
+    const count = await this.getPendingEventCount('fatalTestError');
+    if (count === 0) {
+      return;
+    }
+    for (let i = 0; i < count; i += 1) {
+      // eslint-disable-next-line no-await-in-loop, no-console
+      console.error(await this.waitForEvent('fatalTestError'));
+    }
+    throw new Error('App had fatal test errors');
+  }
+
   public async close(): Promise<void> {
-    await this.app.close();
+    try {
+      await this.checkForFatalTestErrors();
+    } finally {
+      await this.app.close();
+    }
   }
 
   public async getWindow(): Promise<Page> {

ts/util/groupSendEndorsements.ts

@@ -25,6 +25,8 @@ import type { GroupV2MemberType } from '../model-types';
 import { DurationInSeconds } from './durations';
 import { ToastType } from '../types/Toast';
 import * as Errors from '../types/errors';
+import { isTestOrMockEnvironment } from '../environment';
+import { isAlpha } from './version';
 
 export function decodeGroupSendEndorsementResponse({
   groupId,
@@ -116,6 +118,14 @@ export function decodeGroupSendEndorsementResponse({
 const TWO_DAYS = DurationInSeconds.fromDays(2);
 const TWO_HOURS = DurationInSeconds.fromHours(2);
 
+function logServiceIds(list: Iterable<string>) {
+  const items = Array.from(list);
+  if (items.length <= 5) {
+    return items.join(', ');
+  }
+  return `${items.slice(0, 4).join(', ')}, and ${items.length - 4} others`;
+}
+
 export function isValidGroupSendEndorsementsExpiration(
   expiration: number
 ): boolean {
@@ -127,11 +137,11 @@ export function isValidGroupSendEndorsementsExpiration(
 
 export class GroupSendEndorsementState {
   #combinedEndorsement: GroupSendCombinedEndorsementRecord;
-  #memberEndorsements = new Map<
+  #otherMemberEndorsements = new Map<
     ServiceIdString,
     GroupSendMemberEndorsementRecord
   >();
-  #memberEndorsementsAcis = new Set<AciString>();
+  #otherMemberEndorsementsAcis = new Set<AciString>();
   #groupSecretParamsBase64: string;
   #endorsementCache = new WeakMap<Uint8Array, GroupSendEndorsement>();
 
@@ -140,11 +150,15 @@ export class GroupSendEndorsementState {
     groupSecretParamsBase64: string
   ) {
     this.#combinedEndorsement = data.combinedEndorsement;
-    for (const endorsement of data.memberEndorsements) {
-      this.#memberEndorsements.set(endorsement.memberAci, endorsement);
-      this.#memberEndorsementsAcis.add(endorsement.memberAci);
-    }
     this.#groupSecretParamsBase64 = groupSecretParamsBase64;
+
+    const ourAci = window.textsecure.storage.user.getCheckedAci();
+    for (const endorsement of data.memberEndorsements) {
+      if (endorsement.memberAci !== ourAci) {
+        this.#otherMemberEndorsements.set(endorsement.memberAci, endorsement);
+        this.#otherMemberEndorsementsAcis.add(endorsement.memberAci);
+      }
+    }
   }
 
   isSafeExpirationRange(): boolean {
@@ -158,7 +172,7 @@ export class GroupSendEndorsementState {
   }
 
   hasMember(serviceId: ServiceIdString): boolean {
-    return this.#memberEndorsements.has(serviceId);
+    return this.#otherMemberEndorsements.has(serviceId);
   }
 
   #toEndorsement(contents: Uint8Array) {
@@ -173,19 +187,12 @@ export class GroupSendEndorsementState {
   // Strategy 1: Faster when we're sending to most of the group members
   // `combined.byRemoving(combine(difference(members, sends)))`
   #subtractMemberEndorsements(
-    serviceIds: Set<ServiceIdString>
+    difference: Set<ServiceIdString>
   ): GroupSendEndorsement {
-    const difference = this.#memberEndorsementsAcis.difference(serviceIds);
-    const ourAci = window.textsecure.storage.user.getCheckedAci();
-
     const toRemove: Array<GroupSendEndorsement> = [];
-    for (const serviceId of difference) {
-      if (serviceId === ourAci) {
-        // Note: Combined endorsement does not include our aci
-        continue;
-      }
 
-      const memberEndorsement = this.#memberEndorsements.get(serviceId);
+    for (const serviceId of difference) {
+      const memberEndorsement = this.#otherMemberEndorsements.get(serviceId);
       strictAssert(
         memberEndorsement,
         'serializeGroupSendEndorsementFullToken: Missing endorsement'
@@ -204,7 +211,7 @@ export class GroupSendEndorsementState {
     serviceIds: Set<ServiceIdString>
   ): GroupSendEndorsement {
     const memberEndorsements = Array.from(serviceIds).map(serviceId => {
-      const memberEndorsement = this.#memberEndorsements.get(serviceId);
+      const memberEndorsement = this.#otherMemberEndorsements.get(serviceId);
       strictAssert(
         memberEndorsement,
         'serializeGroupSendEndorsementFullToken: Missing endorsement'
@@ -217,17 +224,28 @@ export class GroupSendEndorsementState {
 
   buildToken(serviceIds: Set<ServiceIdString>): GroupSendToken {
     const sendCount = serviceIds.size;
-    const memberCount = this.#memberEndorsements.size;
+    const otherMemberCount = this.#otherMemberEndorsements.size;
-    const logId = `GroupSendEndorsementState.buildToken(${sendCount} of ${memberCount})`;
+    const logId = `GroupSendEndorsementState.buildToken(${sendCount} of ${otherMemberCount})`;
+
+    const missing = serviceIds.difference(this.#otherMemberEndorsementsAcis);
+    if (missing.size !== 0) {
+      throw new Error(
+        `Attempted to build token with memberAcis we don't have endorsements for (${logServiceIds(missing)})`
+      );
+    }
+
+    const difference = this.#otherMemberEndorsementsAcis.difference(serviceIds);
+    log.info(
+      `buildToken: Endorsements without sends ${difference.size}: ${logServiceIds(difference)})`
+    );
 
     let endorsement: GroupSendEndorsement;
-    if (sendCount === memberCount - 1) {
+    if (difference.size === 0) {
       log.info(`${logId}: combinedEndorsement`);
-      // Note: Combined endorsement does not include our aci
       endorsement = this.#toEndorsement(this.#combinedEndorsement.endorsement);
-    } else if (sendCount > (memberCount - 1) / 2) {
+    } else if (difference.size < otherMemberCount / 2) {
       log.info(`${logId}: subtractMemberEndorsements`);
-      endorsement = this.#subtractMemberEndorsements(serviceIds);
+      endorsement = this.#subtractMemberEndorsements(difference);
     } else {
       log.info(`${logId}: combineMemberEndorsements`);
       endorsement = this.#combineMemberEndorsements(serviceIds);
@@ -251,8 +269,13 @@ export class GroupSendEndorsementState {
 
 export function onFailedToSendWithEndorsements(error: Error): void {
   log.error('onFailedToSendWithEndorsements', Errors.toLogFormat(error));
-  window.reduxActions.toast.showToast({
+  if (isTestOrMockEnvironment() || isAlpha(window.getVersion())) {
-    toastType: ToastType.FailedToSendWithEndorsements,
+    window.reduxActions.toast.showToast({
-  });
+      toastType: ToastType.FailedToSendWithEndorsements,
+    });
+  }
+  if (window.SignalCI) {
+    window.SignalCI.handleEvent('fatalTestError', error);
+  }
   assertDev(false, 'We should never fail to send with endorsements');
 }

ts/util/sendToGroup.ts

@@ -72,6 +72,7 @@ import {
 } from './groupSendEndorsements';
 import { maybeUpdateGroup } from '../groups';
 import type { GroupSendToken } from '../types/GroupSendEndorsements';
+import { isAciString } from './isAciString';
 
 const UNKNOWN_RECIPIENT = 404;
 const INCORRECT_AUTH_KEY = 401;
@@ -361,16 +362,18 @@ export async function sendToGroupViaSenderKey(
 
   let groupSendEndorsementState: GroupSendEndorsementState | null = null;
   if (groupId != null) {
+    strictAssert(conversation, 'Must have conversation for endorsements');
+
     const data = await DataReader.getGroupSendEndorsementsData(groupId);
     if (data == null) {
-      onFailedToSendWithEndorsements(
+      if (conversation.isMember(ourAci)) {
-        new Error(
+        onFailedToSendWithEndorsements(
-          `sendToGroupViaSenderKey/${logId}: Missing all endorsements for group`
+          new Error(
-        )
+            `sendToGroupViaSenderKey/${logId}: Missing all endorsements for group`
-      );
+          )
+        );
+      }
     } else {
-      strictAssert(conversation, 'Must have conversation for endorsements');
-
       log.info(
         `sendToGroupViaSenderKey/${logId}: Loaded endorsements for ${data.memberEndorsements.length} members`
       );
@@ -572,9 +575,13 @@ export async function sendToGroupViaSenderKey(
   let accessKeys: Buffer | undefined;
   if (groupSendEndorsementState != null) {
     strictAssert(conversation, 'Must have conversation for endorsements');
-    groupSendToken = groupSendEndorsementState.buildToken(
+    try {
-      new Set(senderKeyRecipients)
+      groupSendToken = groupSendEndorsementState.buildToken(
-    );
+        new Set(senderKeyRecipients)
+      );
+    } catch (error) {
+      onFailedToSendWithEndorsements(error);
+    }
   } else {
     accessKeys = getXorOfAccessKeys(devicesForSenderKey, { story });
   }
@@ -689,6 +696,10 @@ export async function sendToGroupViaSenderKey(
       }
     }
 
+    if (groupSendEndorsementState != null) {
+      onFailedToSendWithEndorsements(error);
+    }
+
     log.error(
       `sendToGroupViaSenderKey/${logId}: Returned unexpected error code: ${
         error.code
@@ -1259,8 +1270,7 @@ function isValidSenderKeyRecipient(
   }
 
   if (groupSendEndorsementState != null) {
-    const memberEndorsement = groupSendEndorsementState.hasMember(serviceId);
+    if (!groupSendEndorsementState.hasMember(serviceId)) {
-    if (memberEndorsement == null) {
       onFailedToSendWithEndorsements(
         new Error(
           `isValidSenderKeyRecipient: Sending to ${serviceId}, missing endorsement`
@@ -1429,6 +1439,12 @@ async function fetchKeysForServiceId(
     'private'
   );
 
+  let useGroupSendEndorsement = isAciString(serviceId);
+  if (!groupSendEndorsementState?.hasMember(serviceId)) {
+    log.error(`fetchKeysForServiceId: ${serviceId} does not have endorsements`);
+    useGroupSendEndorsement = false;
+  }
+
   try {
     // Note: we have no way to make an unrestricted unauthenticated key fetch as part of a
     //   story send, so we hardcode story=false.
@@ -1436,8 +1452,13 @@ async function fetchKeysForServiceId(
       story: false,
     });
 
-    const groupSendToken =
+    let groupSendToken: GroupSendToken | null = null;
-      groupSendEndorsementState?.buildToken(new Set([serviceId])) ?? null;
+
+    if (useGroupSendEndorsement && groupSendEndorsementState != null) {
+      groupSendToken = groupSendEndorsementState.buildToken(
+        new Set([serviceId])
+      );
+    }
 
     const { accessKeyFailed } = await getKeysForServiceId(
       serviceId,
@@ -1456,6 +1477,9 @@ async function fetchKeysForServiceId(
       await DataWriter.updateConversation(emptyConversation.attributes);
     }
   } catch (error: unknown) {
+    if (useGroupSendEndorsement) {
+      onFailedToSendWithEndorsements(error as Error);
+    }
     if (error instanceof UnregisteredUserError) {
       await markServiceIdUnregistered(serviceId);
       return;