From 5c7b98e5c41421c2752d9d17dd6b31c9c621cc97 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 14 May 2018 15:27:34 -0400 Subject: [PATCH] Improve handling of unregistered users. --- .../ViewControllers/DebugUI/DebugUIMessages.m | 2 +- .../translations/en.lproj/Localizable.strings | 3 ++ .../src/Contacts/ContactsUpdater.m | 3 ++ .../src/Contacts/SignalRecipient.m | 9 +++++- .../src/Messages/Interactions/TSInfoMessage.h | 10 ++++-- .../src/Messages/Interactions/TSInfoMessage.m | 32 ++++++++++++++++--- .../src/Messages/OWSMessageSender.m | 16 ++++++++-- 7 files changed, 64 insertions(+), 11 deletions(-) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index 1b02fe789..d2e32ed5e 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m @@ -3452,7 +3452,7 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac createdByRemoteName:@"Alice"]]; } - [result addObject:[TSInfoMessage userNotRegisteredMessageInThread:thread]]; + [result addObject:[TSInfoMessage userNotRegisteredMessageInThread:thread recipientId:@"+19174054215"]]; [result addObject:[[TSInfoMessage alloc] initWithTimestamp:[NSDate ows_millisecondTimeStamp] inThread:thread diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index 257cba8e1..ed1421f23 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -857,6 +857,9 @@ /* No comment provided by engineer. */ "ERROR_MESSAGE_WRONG_TRUSTED_IDENTITY_KEY" = "Safety number changed. Tap to verify."; +/* Format string for 'unregistered user' error. Embeds {{the unregistered user's signal id}}. */ +"ERROR_UNREGISTERED_USER_FORMAT" = "Unregistered User: %@"; + /* action sheet header when re-sending message which failed because of too many attempts */ "FAILED_SENDING_BECAUSE_RATE_LIMIT" = "Too many failures with this contact. Please try again shortly."; diff --git a/SignalServiceKit/src/Contacts/ContactsUpdater.m b/SignalServiceKit/src/Contacts/ContactsUpdater.m index e7138d723..8d972f7c0 100644 --- a/SignalServiceKit/src/Contacts/ContactsUpdater.m +++ b/SignalServiceKit/src/Contacts/ContactsUpdater.m @@ -58,6 +58,9 @@ NS_ASSUME_NONNULL_BEGIN dispatch_semaphore_signal(sema); } failure:^(NSError *lookupError) { + DDLogError( + @"%@ Could not find recipient for recipientId: %@, error: %@.", self.logTag, identifier, lookupError); + retainedError = lookupError; dispatch_semaphore_signal(sema); }]; diff --git a/SignalServiceKit/src/Contacts/SignalRecipient.m b/SignalServiceKit/src/Contacts/SignalRecipient.m index 3e08e4558..d206ea1bf 100644 --- a/SignalServiceKit/src/Contacts/SignalRecipient.m +++ b/SignalServiceKit/src/Contacts/SignalRecipient.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "SignalRecipient.h" @@ -110,6 +110,13 @@ NS_ASSUME_NONNULL_BEGIN return [self.recipientId compare:other.recipientId]; } +- (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction +{ + [super saveWithTransaction:transaction]; + + DDLogVerbose(@"%@ saved signal recipient: %@", self.logTag, self.recipientId); +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.h b/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.h index c870431ea..03d1e7d05 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.h @@ -23,10 +23,11 @@ typedef NS_ENUM(NSInteger, TSInfoMessageType) { TSInfoMessageAddGroupToProfileWhitelistOffer, }; -+ (instancetype)userNotRegisteredMessageInThread:(TSThread *)thread; ++ (instancetype)userNotRegisteredMessageInThread:(TSThread *)thread recipientId:(NSString *)recipientId; @property (atomic, readonly) TSInfoMessageType messageType; -@property (atomic, readonly) NSString *customMessage; +@property (atomic, readonly, nullable) NSString *customMessage; +@property (atomic, readonly, nullable) NSString *unregisteredRecipientId; - (instancetype)initMessageWithTimestamp:(uint64_t)timestamp inThread:(nullable TSThread *)thread @@ -48,6 +49,11 @@ typedef NS_ENUM(NSInteger, TSInfoMessageType) { messageType:(TSInfoMessageType)infoMessage customMessage:(NSString *)customMessage; +- (instancetype)initWithTimestamp:(uint64_t)timestamp + inThread:(TSThread *)thread + messageType:(TSInfoMessageType)infoMessage + unregisteredRecipientId:(NSString *)unregisteredRecipientId; + - (instancetype)initWithTimestamp:(uint64_t)timestamp inThread:(nullable TSThread *)thread messageBody:(nullable NSString *)body diff --git a/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m b/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m index 4ee76bb4d..82c6c7b4e 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m @@ -72,7 +72,8 @@ NSUInteger TSInfoMessageSchemaVersion = 1; - (instancetype)initWithTimestamp:(uint64_t)timestamp inThread:(TSThread *)thread messageType:(TSInfoMessageType)infoMessage - customMessage:(NSString *)customMessage { + customMessage:(NSString *)customMessage +{ self = [self initWithTimestamp:timestamp inThread:thread messageType:infoMessage]; if (self) { _customMessage = customMessage; @@ -80,11 +81,27 @@ NSUInteger TSInfoMessageSchemaVersion = 1; return self; } -+ (instancetype)userNotRegisteredMessageInThread:(TSThread *)thread +- (instancetype)initWithTimestamp:(uint64_t)timestamp + inThread:(TSThread *)thread + messageType:(TSInfoMessageType)infoMessage + unregisteredRecipientId:(NSString *)unregisteredRecipientId +{ + self = [self initWithTimestamp:timestamp inThread:thread messageType:infoMessage]; + if (self) { + _unregisteredRecipientId = unregisteredRecipientId; + } + return self; +} + ++ (instancetype)userNotRegisteredMessageInThread:(TSThread *)thread recipientId:(NSString *)recipientId { + OWSAssert(thread); + OWSAssert(recipientId); + return [[self alloc] initWithTimestamp:[NSDate ows_millisecondTimeStamp] inThread:thread - messageType:TSInfoMessageUserNotRegistered]; + messageType:TSInfoMessageUserNotRegistered + unregisteredRecipientId:recipientId]; } - (OWSInteractionType)interactionType @@ -100,7 +117,14 @@ NSUInteger TSInfoMessageSchemaVersion = 1; case TSInfoMessageTypeUnsupportedMessage: return NSLocalizedString(@"UNSUPPORTED_ATTACHMENT", nil); case TSInfoMessageUserNotRegistered: - return NSLocalizedString(@"CONTACT_DETAIL_COMM_TYPE_INSECURE", nil); + if (self.unregisteredRecipientId.length > 0) { + return [NSString stringWithFormat:NSLocalizedString(@"ERROR_UNREGISTERED_USER_FORMAT", + @"Format string for 'unregistered user' error. Embeds {{the " + @"unregistered user's signal id}}."), + self.unregisteredRecipientId]; + } else { + return NSLocalizedString(@"CONTACT_DETAIL_COMM_TYPE_INSECURE", nil); + } case TSInfoMessageTypeGroupQuit: return NSLocalizedString(@"GROUP_YOU_LEFT", nil); case TSInfoMessageTypeGroupUpdate: diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 5863a5450..9b5a2d2a9 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -448,8 +448,12 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; }); } -- (NSArray *)getRecipientsForRecipientIds:(NSArray *)recipientIds error:(NSError **)error +- (NSArray *)signalRecipientsForRecipientIds:(NSArray *)recipientIds + message:(TSOutgoingMessage *)message + error:(NSError **)error { + OWSAssert(recipientIds); + OWSAssert(message); OWSAssert(error); *error = nil; @@ -465,6 +469,12 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; SignalRecipient *newRecipient = [self.contactsUpdater synchronousLookup:recipientId error:error]; if (newRecipient) { [recipients addObject:newRecipient]; + } else { + DDLogWarn(@"%@ No SignalRecipient for recipientId: %@", self.logTag, recipientId); + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + // Mark this recipient as "skipped". + [message updateWithSkippedRecipient:recipientId transaction:transaction]; + }]; } } } @@ -544,7 +554,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; NSError *error; NSArray *recipients = - [self getRecipientsForRecipientIds:sendingRecipientIds.allObjects error:&error]; + [self signalRecipientsForRecipientIds:sendingRecipientIds.allObjects message:message error:&error]; if (recipients.count == 0) { if (!error) { @@ -753,7 +763,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; } [recipient removeWithTransaction:transaction]; - [[TSInfoMessage userNotRegisteredMessageInThread:thread] + [[TSInfoMessage userNotRegisteredMessageInThread:thread recipientId:recipient.recipientId] saveWithTransaction:transaction]; }]; }