Keep profileKey in situations where it didn't work; still fail over

ts/models/conversations.ts

@@ -2341,6 +2341,9 @@ export class ConversationModel extends window.Backbone
   async addMessageRequestResponseEventMessage(
     event: MessageRequestResponseEvent
   ): Promise<void> {
+    const idForLogging = getConversationIdForLogging(this.attributes);
+    log.info(`addMessageRequestResponseEventMessage/${idForLogging}: ${event}`);
+
     const timestamp = Date.now();
     const lastMessageTimestamp =
       // Fallback to `timestamp` since `lastMessageReceivedAtMs` is new
@@ -5018,7 +5021,7 @@ export class ConversationModel extends window.Backbone
   }
 
   async setProfileKey(
-    profileKey: string | undefined,
+    profileKey: string,
     {
       viaStorageServiceSync = false,
       reason,
@@ -5028,6 +5031,10 @@ export class ConversationModel extends window.Backbone
       profileKey == null || profileKey.length > 0,
       'setProfileKey: Profile key cannot be an empty string'
     );
+    if (profileKey === undefined) {
+      log.warn('setProfileKey: Refusing to set an undefined profileKey');
+      return false;
+    }
 
     const oldProfileKey = this.get('profileKey');
 
@@ -5077,7 +5084,7 @@ export class ConversationModel extends window.Backbone
     // If our profile key was cleared above, we don't tell our linked devices about it.
     //   We want linked devices to tell us what it should be, instead of telling them to
     //   erase their local value.
-    if (!viaStorageServiceSync && profileKey) {
+    if (!viaStorageServiceSync) {
       this.captureChange('profileKey');
     }
 
@@ -5151,10 +5158,8 @@ export class ConversationModel extends window.Backbone
     const profileKeyVersion = deriveProfileKeyVersion(profileKey, serviceId);
     if (!profileKeyVersion) {
       log.warn(
-        'deriveProfileKeyVersion: Failed to derive profile key version, ' +
+        'deriveProfileKeyVersion: Failed to derive profile key version, return nothing.'
-          'clearing profile key.'
       );
-      void this.setProfileKey(undefined, { reason: 'deriveProfileKeyVersion' });
       return;
     }
 
@@ -5189,32 +5194,6 @@ export class ConversationModel extends window.Backbone
     await DataWriter.updateConversation(this.attributes);
   }
 
-  async removeLastProfile(
-    oldValue: ConversationLastProfileType | undefined
-  ): Promise<void> {
-    // Atomic updates only
-    if (this.get('lastProfile') !== oldValue) {
-      return;
-    }
-
-    log.warn(
-      'ConversationModel.removeLastProfile: called for',
-      this.idForLogging()
-    );
-
-    this.set({
-      lastProfile: undefined,
-
-      // We don't have any knowledge of profile anymore. Drop all associated
-      // data.
-      about: undefined,
-      aboutEmoji: undefined,
-      profileAvatar: undefined,
-    });
-
-    await DataWriter.updateConversation(this.attributes);
-  }
-
   hasMember(serviceId: ServiceIdString): boolean {
     const members = this.getMembers();
 

ts/services/profiles.ts

@@ -292,11 +292,13 @@ async function buildProfileFetchOptions({
   lastProfile,
   clientZkProfileCipher,
   groupId,
+  options,
 }: {
   conversation: ConversationModel;
   lastProfile: ConversationLastProfileType | null;
   clientZkProfileCipher: ClientZkProfileOperations;
   groupId: string | null;
+  options: { ignoreProfileKey: boolean; ignoreGroupSendToken: boolean };
 }): Promise<ProfileFetchOptions.Auth | ProfileFetchOptions.Unauth> {
   const logId = `buildGetProfileOptions(${conversation.idForLogging()})`;
 
@@ -310,12 +312,12 @@ async function buildProfileFetchOptions({
   const accessKey = conversation.get('accessKey');
   const serviceId = conversation.getCheckedServiceId('getProfile');
 
-  if (profileKey) {
+  if (
-    strictAssert(
+    profileKey &&
-      profileKeyVersion != null && accessKey != null,
+    profileKeyVersion &&
-      `${logId}: profileKeyVersion and accessKey are derived from profileKey`
+    accessKey &&
-    );
+    !options.ignoreProfileKey
-
+  ) {
     if (!conversation.hasProfileKeyCredentialExpired()) {
       log.info(`${logId}: using unexpired profile key credential`);
       return {
@@ -351,15 +353,12 @@ async function buildProfileFetchOptions({
     };
   }
 
-  strictAssert(
+  // If we're ignoring profileKey, try getting the versioned profile with lastProfile.
-    accessKey == null,
+  // Note: No access key, since this is almost guaranteed not to be their current profile.
-    `${logId}: accessKey have to be absent because there is no profileKey`
+  // Also, we can't try the group send token here because the versioned profile can't be
-  );
+  // decrypted without an up to date profile key.
-
-  // If we have a `lastProfile`, try getting the versioned profile with auth.
-  // Note: We can't try the group send token here because the versioned profile
-  // can't be decrypted without an up to date profile key.
   if (
+    options.ignoreProfileKey &&
     lastProfile != null &&
     lastProfile.profileKey != null &&
     lastProfile.profileKeyVersion != null
@@ -379,7 +378,7 @@ async function buildProfileFetchOptions({
   }
 
   // Fallback to group send tokens for unversioned profiles
-  if (groupId != null) {
+  if (groupId != null && !options.ignoreGroupSendToken) {
     log.info(`${logId}: fetching group endorsements`);
     let result = await maybeCreateGroupSendEndorsementState(groupId, false);
 
@@ -456,7 +455,14 @@ function getFetchOptionsLabel(
 
 async function doGetProfile(
   c: ConversationModel,
-  groupId: string | null
+  groupId: string | null,
+  {
+    ignoreProfileKey,
+    ignoreGroupSendToken,
+  }: {
+    ignoreProfileKey: boolean;
+    ignoreGroupSendToken: boolean;
+  } = { ignoreProfileKey: false, ignoreGroupSendToken: false }
 ): Promise<void> {
   const logId = groupId
     ? `getProfile(${c.idForLogging()} in groupv2(${groupId}))`
@@ -482,10 +488,8 @@ async function doGetProfile(
 
   const serviceId = c.getCheckedServiceId('getProfile');
 
-  // Step #: Grab the profile key and version we last were successful decrypting with
+  // Step #: Grab the profile key and version we last were successful decrypting with.
-  // `lastProfile` is saved at the end of `doGetProfile` after successfully decrypting.
+  // We'll use it in case we've failed to fetch with our `profileKey`.
-  // `lastProfile` is used in case the `profileKey` was cleared because of a 401/403.
-  // `lastProfile` is cleared when we get a 404 fetching a profile.
   const lastProfile = c.get('lastProfile');
 
   // Step #: Build the request options we will use for fetching and decrypting the profile
@@ -494,6 +498,10 @@ async function doGetProfile(
     lastProfile: lastProfile ?? null,
     clientZkProfileCipher,
     groupId,
+    options: {
+      ignoreProfileKey,
+      ignoreGroupSendToken,
+    },
   });
   const { request } = options;
 
@@ -523,43 +531,30 @@ async function doGetProfile(
           // Fallback from failed unauth (access key) request
           if (request.accessKey != null) {
             log.warn(
-              `${logId}: Got ${error.code} when using access key, removing profileKey and retrying`
+              `${logId}: Got ${error.code} when using access key, failing over to lastProfile`
             );
-            await c.setProfileKey(undefined, {
-              reason: 'doGetProfile/accessKey/401+403',
-            });
 
-            // Retry fetch using last known profileKeyVersion or fetch
+            // Record that the accessKey we have in the conversation is invalid
-            // unversioned profile.
+            const sealedSender = c.get('sealedSender');
-            return doGetProfile(c, groupId);
+            if (sealedSender !== SEALED_SENDER.DISABLED) {
+              c.set('sealedSender', SEALED_SENDER.DISABLED);
+            }
+
+            // Retry fetch using last known profileKey or fetch unversioned profile.
+            return doGetProfile(c, groupId, {
+              ignoreProfileKey: true,
+              ignoreGroupSendToken,
+            });
           }
 
           // Fallback from failed unauth (group send token) request
           if (request.groupSendToken != null) {
             log.warn(`${logId}: Got ${error.code} when using group send token`);
-            return doGetProfile(c, null);
+            return doGetProfile(c, null, {
-          }
+              ignoreProfileKey,
-        }
+              ignoreGroupSendToken: true,
-
-        // Step #: Record if the accessKey we have in the conversation is valid
-        const sealedSender = c.get('sealedSender');
-        if (
-          sealedSender === SEALED_SENDER.ENABLED ||
-          sealedSender === SEALED_SENDER.UNRESTRICTED
-        ) {
-          if (!isMe(c.attributes)) {
-            log.warn(
-              `${logId}: Got ${error.code} when using accessKey, removing profileKey`
-            );
-            await c.setProfileKey(undefined, {
-              reason: 'doGetProfile/accessKey/401+403',
             });
           }
-        } else if (sealedSender === SEALED_SENDER.UNKNOWN) {
-          log.warn(
-            `${logId}: Got ${error.code} fetching profile, setting sealedSender = DISABLED`
-          );
-          c.set('sealedSender', SEALED_SENDER.DISABLED);
         }
 
         // TODO: Is it safe to ignore these errors?
@@ -571,10 +566,8 @@ async function doGetProfile(
         log.info(`${logId}: Profile not found`);
 
         c.set('profileLastFetchedAt', Date.now());
-        // Note: Writes to DB:
-        await c.removeLastProfile(lastProfile);
 
-        if (!isVersioned) {
+        if (!isVersioned || ignoreProfileKey) {
           log.info(`${logId}: Marking conversation unregistered`);
           c.setUnregistered();
         }

ts/util/handleMessageSend.ts

@@ -108,19 +108,7 @@ function processError(error: unknown): void {
       'private'
     );
     if (error.code === 401 || error.code === 403) {
-      if (
+      if (conversation.get('sealedSender') !== SEALED_SENDER.DISABLED) {
-        conversation.get('sealedSender') === SEALED_SENDER.ENABLED ||
-        conversation.get('sealedSender') === SEALED_SENDER.UNRESTRICTED
-      ) {
-        log.warn(
-          `handleMessageSend: Got 401/403 for ${conversation.idForLogging()}, removing profile key`
-        );
-
-        void conversation.setProfileKey(undefined, {
-          reason: 'handleMessageSend/processError',
-        });
-      }
-      if (conversation.get('sealedSender') === SEALED_SENDER.UNKNOWN) {
         log.warn(
           `handleMessageSend: Got 401/403 for ${conversation.idForLogging()}, setting sealedSender = DISABLED`
         );