From b0dfae7974717f618cf4ff20b15289b5ef3caa2a Mon Sep 17 00:00:00 2001 From: Mikunj Date: Mon, 2 Dec 2019 09:49:16 +1100 Subject: [PATCH] Fix multi-device profile picture handling. Fixed note to self. Enabled removal of avatar. --- .../AppSettings/AppSettingsViewController.m | 8 +-- Signal/src/ViewControllers/AvatarViewHelper.m | 3 +- .../HomeView/HomeViewController.m | 4 +- SignalMessaging/profiles/OWSProfileManager.m | 61 ++++++++----------- SignalMessaging/profiles/OWSUserProfile.m | 5 +- .../utils/OWSContactAvatarBuilder.m | 3 +- SignalServiceKit/src/Contacts/TSThread.m | 6 +- .../Public Chat/LokiPublicChatPoller.swift | 6 +- .../src/Messages/OWSMessageManager.m | 27 ++++---- .../src/Protocols/ProfileManagerProtocol.h | 2 - .../src/TestUtils/OWSFakeProfileManager.m | 5 -- 11 files changed, 57 insertions(+), 73 deletions(-) diff --git a/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m b/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m index 11f0a2834..cd19fd76b 100644 --- a/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m +++ b/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m @@ -314,6 +314,7 @@ - (UITableViewCell *)profileHeaderCell { NSString *masterDeviceHexEncodedPublicKey = [NSUserDefaults.standardUserDefaults stringForKey:@"masterDeviceHexEncodedPublicKey"]; + NSString *hexEncodedPublicKey = masterDeviceHexEncodedPublicKey ?: TSAccountManager.localNumber; BOOL isMasterDevice = (masterDeviceHexEncodedPublicKey == nil); UITableViewCell *cell = [OWSTableItem newCell]; @@ -321,7 +322,7 @@ cell.contentView.preservesSuperviewLayoutMargins = YES; cell.selectionStyle = UITableViewCellSelectionStyleNone; - UIImage *_Nullable localProfileAvatarImage = [OWSProfileManager.sharedManager localProfileAvatarImage]; + UIImage *_Nullable localProfileAvatarImage = [OWSProfileManager.sharedManager profileAvatarForRecipientId:hexEncodedPublicKey]; UIImage *avatarImage = (localProfileAvatarImage ?: [[[OWSContactAvatarBuilder alloc] initForLocalUserWithDiameter:kLargeAvatarSize] buildDefaultImage]); OWSAssertDebug(avatarImage); @@ -334,7 +335,7 @@ [avatarView autoSetDimension:ALDimensionHeight toSize:kLargeAvatarSize]; avatarView.contactID = OWSIdentityManager.sharedManager.identityKeyPair.hexEncodedPublicKey; - if (isMasterDevice && !localProfileAvatarImage) { + if (isMasterDevice && !OWSProfileManager.sharedManager.localProfileAvatarImage) { UIImage *cameraImage = [UIImage imageNamed:@"settings-avatar-camera"]; UIImageView *cameraImageView = [[UIImageView alloc] initWithImage:cameraImage]; [cell.contentView addSubview:cameraImageView]; @@ -348,7 +349,7 @@ [nameView autoPinLeadingToTrailingEdgeOfView:avatarView offset:16.f]; UILabel *titleLabel = [UILabel new]; - NSString *_Nullable localProfileName = [OWSProfileManager.sharedManager localProfileName]; + NSString *_Nullable localProfileName = [OWSProfileManager.sharedManager profileNameForRecipientId:hexEncodedPublicKey]; if (localProfileName.length > 0) { titleLabel.text = localProfileName; titleLabel.textColor = [Theme primaryColor]; @@ -370,7 +371,6 @@ UILabel *subtitleLabel = [UILabel new]; subtitleLabel.textColor = [Theme secondaryColor]; subtitleLabel.font = [UIFont ows_regularFontWithSize:kSubtitlePointSize]; - NSString *hexEncodedPublicKey = masterDeviceHexEncodedPublicKey ?: TSAccountManager.localNumber; subtitleLabel.attributedText = [[NSAttributedString alloc] initWithString:hexEncodedPublicKey]; subtitleLabel.lineBreakMode = NSLineBreakByTruncatingTail; [nameView addSubview:subtitleLabel]; diff --git a/Signal/src/ViewControllers/AvatarViewHelper.m b/Signal/src/ViewControllers/AvatarViewHelper.m index 33ca13b42..649fbbeec 100644 --- a/Signal/src/ViewControllers/AvatarViewHelper.m +++ b/Signal/src/ViewControllers/AvatarViewHelper.m @@ -58,8 +58,7 @@ NS_ASSUME_NONNULL_BEGIN [self.delegate clearAvatar]; }]; - // TODO: enable this once we support removing avatars (as opposed to replacing) - // [actionSheet addAction:clearAction]; + [actionSheet addAction:clearAction]; } [self.delegate.fromViewController presentAlert:actionSheet]; diff --git a/Signal/src/ViewControllers/HomeView/HomeViewController.m b/Signal/src/ViewControllers/HomeView/HomeViewController.m index c7bd82edc..1563e6c5a 100644 --- a/Signal/src/ViewControllers/HomeView/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeView/HomeViewController.m @@ -759,7 +759,9 @@ typedef NS_ENUM(NSInteger, HomeViewControllerSection) { UIBarButtonItem *settingsButton; if (SYSTEM_VERSION_GREATER_THAN_OR_EQUAL_TO(11, 0)) { const NSUInteger kAvatarSize = 28; - UIImage *_Nullable localProfileAvatarImage = [OWSProfileManager.sharedManager localProfileAvatarImage]; + NSString *masterDeviceHexEncodedPublicKey = [NSUserDefaults.standardUserDefaults stringForKey:@"masterDeviceHexEncodedPublicKey"]; + NSString *hexEncodedPublicKey = masterDeviceHexEncodedPublicKey ?: TSAccountManager.localNumber; + UIImage *_Nullable localProfileAvatarImage = [OWSProfileManager.sharedManager profileAvatarForRecipientId:hexEncodedPublicKey]; UIImage *avatarImage = (localProfileAvatarImage ?: [[[OWSContactAvatarBuilder alloc] initForLocalUserWithDiameter:kAvatarSize] buildDefaultImage]); OWSAssertDebug(avatarImage); diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 5e0fa88e5..1270663b4 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -341,22 +341,6 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); } } -- (void)updateUserProfileWithDisplayName:(nullable NSString*)displayName transaction:(YapDatabaseReadWriteTransaction *)transaction -{ - [self.localUserProfile updateWithProfileName:displayName transaction:transaction]; -} - -- (void)updateUserProfileKeyData:(NSData *)profileKeyData avatarURL:(nullable NSString *)avatarURL transaction:(YapDatabaseReadWriteTransaction *)transaction { - OWSAES256Key *profileKey = [OWSAES256Key keyWithData:profileKeyData]; - if (profileKey != nil) { - [self.localUserProfile updateWithProfileKey:profileKey transaction:transaction completion:^{ - [self.localUserProfile updateWithAvatarUrlPath:avatarURL transaction:transaction completion:^{ - [self downloadAvatarForUserProfile:self.localUserProfile]; - }]; - }]; - } -} - - (nullable NSString *)profilePictureURL { return self.localUserProfile.avatarUrlPath; @@ -422,30 +406,35 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); // We always want to encrypt a profile with a new profile key // This ensures that other users know that our profile picture was updated OWSAES256Key *newProfileKey = [OWSAES256Key generateRandomKey]; - NSData *_Nullable encryptedAvatarData; + if (avatarData) { - encryptedAvatarData = [self encryptProfileData:avatarData profileKey:newProfileKey]; + NSData *encryptedAvatarData = [self encryptProfileData:avatarData profileKey:newProfileKey]; OWSAssertDebug(encryptedAvatarData.length > 0); - } - - [[LKStorageAPI setProfilePicture:encryptedAvatarData] - .thenOn(dispatch_get_main_queue(), ^(NSString *url) { - [self.localUserProfile updateWithProfileKey:newProfileKey dbConnection:self.dbConnection completion:^{ - successBlock(url); - }]; - }) - .catchOn(dispatch_get_main_queue(), ^(id result) { - // There appears to be a bug in PromiseKit that sometimes causes catchOn - // to be invoked with the fulfilled promise's value as the error. The below - // is a quick and dirty workaround. - if ([result isKindOfClass:NSString.class]) { + + [[LKStorageAPI setProfilePicture:encryptedAvatarData] + .thenOn(dispatch_get_main_queue(), ^(NSString *url) { [self.localUserProfile updateWithProfileKey:newProfileKey dbConnection:self.dbConnection completion:^{ - successBlock(result); + successBlock(url); }]; - } else { - failureBlock(result); - } - }) retainUntilComplete]; + }) + .catchOn(dispatch_get_main_queue(), ^(id result) { + // There appears to be a bug in PromiseKit that sometimes causes catchOn + // to be invoked with the fulfilled promise's value as the error. The below + // is a quick and dirty workaround. + if ([result isKindOfClass:NSString.class]) { + [self.localUserProfile updateWithProfileKey:newProfileKey dbConnection:self.dbConnection completion:^{ + successBlock(result); + }]; + } else { + failureBlock(result); + } + }) retainUntilComplete]; + } else { + // Update our profile key and set the url to nil if avatar data is nil + [self.localUserProfile updateWithProfileKey:newProfileKey dbConnection:self.dbConnection completion:^{ + successBlock(nil); + }]; + } }); } diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m index ca97889fd..a33538518 100644 --- a/SignalMessaging/profiles/OWSUserProfile.m +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -234,8 +234,9 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; if (!didChange) { return; } - - BOOL isLocalUserProfile = [self.recipientId isEqualToString:kLocalProfileUniqueId]; + + NSString *masterDeviceHexEncodedPublicKey = [NSUserDefaults.standardUserDefaults stringForKey:@"masterDeviceHexEncodedPublicKey"]; + BOOL isLocalUserProfile = [self.recipientId isEqualToString:kLocalProfileUniqueId] || (masterDeviceHexEncodedPublicKey != nil && [self.recipientId isEqualToString:masterDeviceHexEncodedPublicKey]); dispatch_async(dispatch_get_main_queue(), ^{ if (isLocalUserProfile) { diff --git a/SignalMessaging/utils/OWSContactAvatarBuilder.m b/SignalMessaging/utils/OWSContactAvatarBuilder.m index 54e5bec7a..eb6e9b43a 100644 --- a/SignalMessaging/utils/OWSContactAvatarBuilder.m +++ b/SignalMessaging/utils/OWSContactAvatarBuilder.m @@ -93,7 +93,8 @@ NS_ASSUME_NONNULL_BEGIN - (nullable UIImage *)buildSavedImage { - if ([self.signalId isEqualToString:TSAccountManager.localNumber]) { + NSString *masterDeviceHexEncodedPublicKey = [NSUserDefaults.standardUserDefaults stringForKey:@"masterDeviceHexEncodedPublicKey"]; + if ([self.signalId isEqualToString:TSAccountManager.localNumber] || (masterDeviceHexEncodedPublicKey != nil && [self.signalId isEqualToString:masterDeviceHexEncodedPublicKey])) { NSString *noteToSelfCacheKey = [NSString stringWithFormat:@"%@:note-to-self", self.cacheKey]; UIImage *_Nullable cachedAvatar = [OWSContactAvatarBuilder.contactsManager.avatarCache imageForKey:noteToSelfCacheKey diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index 05703ab35..47609049a 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -211,8 +211,10 @@ ConversationColorName const kConversationColorName_Default = ConversationColorNa if (!IsNoteToSelfEnabled()) { return NO; } - return (!self.isGroupThread && self.contactIdentifier != nil && - [self.contactIdentifier isEqualToString:self.tsAccountManager.localNumber]); + NSString *localNumber = self.tsAccountManager.localNumber; + NSString *masterDeviceHexEncodedPublicKey = [NSUserDefaults.standardUserDefaults stringForKey:@"masterDeviceHexEncodedPublicKey"]; + bool isOurNumber = [self.contactIdentifier isEqualToString:localNumber] || (masterDeviceHexEncodedPublicKey != nil && [self.contactIdentifier isEqualToString:masterDeviceHexEncodedPublicKey]); + return (!self.isGroupThread && self.contactIdentifier != nil && isOurNumber); } #pragma mark - To be subclassed. diff --git a/SignalServiceKit/src/Loki/API/Public Chat/LokiPublicChatPoller.swift b/SignalServiceKit/src/Loki/API/Public Chat/LokiPublicChatPoller.swift index a883e15c7..a0fca72e4 100644 --- a/SignalServiceKit/src/Loki/API/Public Chat/LokiPublicChatPoller.swift +++ b/SignalServiceKit/src/Loki/API/Public Chat/LokiPublicChatPoller.swift @@ -166,9 +166,11 @@ public final class LokiPublicChatPoller : NSObject { // If we got a message from our primary device then we should use its avatar if let avatar = message.avatar, masterHexEncodedPublicKey == message.hexEncodedPublicKey { - SSKEnvironment.shared.profileManager.updateUserProfile(withDisplayName: message.displayName, transaction: transaction) - SSKEnvironment.shared.profileManager.updateUserProfileKeyData(avatar.profileKey, avatarURL: avatar.url, transaction: transaction) + if (message.displayName.count > 0) { + SSKEnvironment.shared.profileManager.updateProfileForContact(withID: masterHexEncodedPublicKey!, displayName: message.displayName, with: transaction) + } SSKEnvironment.shared.profileManager.updateService(withProfileName: message.displayName, avatarUrl: avatar.url) + SSKEnvironment.shared.profileManager.setProfileKeyData(avatar.profileKey, forRecipientId: masterHexEncodedPublicKey!, avatarURL: avatar.url) } } } diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 6f1ed547d..ba415af21 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -468,7 +468,8 @@ NS_ASSUME_NONNULL_BEGIN // Set any profile information if (contentProto.dataMessage) { - [self handleLokiProfileForUserIfNeeded:contentProto.dataMessage transaction:transaction]; + [self handleProfileNameUpdateIfNeeded:contentProto.dataMessage recipientId:masterHexEncodedPublicKey transaction:transaction]; + [self handleProfileKeyUpdateIfNeeded:contentProto.dataMessage recipientId:masterHexEncodedPublicKey]; } } else if (slaveSignature != nil) { // Request OWSLogInfo(@"[Loki] Received a device linking request from: %@", envelope.source); // Not slaveHexEncodedPublicKey @@ -925,7 +926,8 @@ NS_ASSUME_NONNULL_BEGIN // Loki: Try to update using the provided profile if (wasSentByMasterDevice) { - [self handleLokiProfileForUserIfNeeded:syncMessage.sent.message transaction:transaction]; + [self handleProfileNameUpdateIfNeeded:syncMessage.sent.message recipientId:masterHexEncodedPublicKey transaction:transaction]; + [self handleProfileKeyUpdateIfNeeded:syncMessage.sent.message recipientId:masterHexEncodedPublicKey]; } OWSIncomingSentMessageTranscript *transcript = @@ -1061,18 +1063,6 @@ NS_ASSUME_NONNULL_BEGIN } } -- (void)handleLokiProfileForUserIfNeeded:(SSKProtoDataMessage *)dataMessage transaction:(YapDatabaseReadWriteTransaction *)transaction { - if (dataMessage.profile != nil) { - SSKProtoDataMessageLokiProfile *profile = dataMessage.profile; - NSString *displayName = profile.displayName; - NSString *profilePictureURL = profile.profilePicture; - [self.profileManager updateUserProfileWithDisplayName:displayName transaction:transaction]; - if ([dataMessage hasProfileKey]) { - [self.profileManager updateUserProfileKeyData:dataMessage.profileKey avatarURL:profilePictureURL transaction:transaction]; - } - } -} - - (void)handleEndSessionMessageWithEnvelope:(SSKProtoEnvelope *)envelope dataMessage:(SSKProtoDataMessage *)dataMessage transaction:(YapDatabaseReadWriteTransaction *)transaction @@ -1392,8 +1382,7 @@ NS_ASSUME_NONNULL_BEGIN [TSContactThread getOrCreateThreadWithContactId:hexEncodedPublicKey transaction:transaction]; // Only set the display name here, the logic for updating profile pictures is handled when we're setting profile key - NSString *displayName = dataMessage.profile.displayName; - [self.profileManager updateProfileForContactWithID:thread.contactIdentifier displayName:displayName with:transaction]; + [self handleProfileNameUpdateIfNeeded:dataMessage recipientId:thread.contactIdentifier transaction:transaction]; switch (dataMessage.group.type) { case SSKProtoGroupContextTypeUpdate: { @@ -1633,6 +1622,12 @@ NS_ASSUME_NONNULL_BEGIN } } +- (void)handleProfileNameUpdateIfNeeded:(SSKProtoDataMessage *)dataMessage recipientId:(NSString *)recipientId transaction:(YapDatabaseReadWriteTransaction *)transaction { + if (dataMessage.profile != nil) { + [self.profileManager updateProfileForContactWithID:recipientId displayName:dataMessage.profile.displayName with:transaction]; + } +} + - (void)handleProfileKeyUpdateIfNeeded:(SSKProtoDataMessage *)dataMessage recipientId:(NSString *)recipientId { if ([dataMessage hasProfileKey]) { NSData *profileKey = [dataMessage profileKey]; diff --git a/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h b/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h index 5f2cebc44..f2d13a742 100644 --- a/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h +++ b/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h @@ -32,8 +32,6 @@ NS_ASSUME_NONNULL_BEGIN - (void)fetchProfileForRecipientId:(NSString *)recipientId; -- (void)updateUserProfileWithDisplayName:(nullable NSString*)displayName transaction:(YapDatabaseReadWriteTransaction *)transaction; -- (void)updateUserProfileKeyData:(NSData *)profileKeyData avatarURL:(nullable NSString *)avatarURL transaction:(YapDatabaseReadWriteTransaction *)transaction; - (void)updateProfileForContactWithID:(NSString *)contactID displayName:(NSString *)displayName with:(YapDatabaseReadWriteTransaction *)transaction; - (void)updateServiceWithProfileName:(nullable NSString *)localProfileName avatarUrl:(nullable NSString *)avatarURL; diff --git a/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m b/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m index de7cf2044..ac8bd1446 100644 --- a/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m +++ b/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m @@ -48,11 +48,6 @@ NS_ASSUME_NONNULL_BEGIN return _localProfileKey; } -- (void)updateUserProfileWithDisplayName:(nullable NSString*)displayName transaction:(YapDatabaseReadWriteTransaction *)transaction -{ - // Do nothing -} - - (void)setProfileKeyData:(NSData *)profileKey forRecipientId:(NSString *)recipientId { OWSAES256Key *_Nullable key = [OWSAES256Key keyWithData:profileKey];