From 4e995b76f3b3dfa4806a898504eb870274f67a9c Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 7 Jun 2017 14:27:24 -0400 Subject: [PATCH] Don't try to share file to untrusted identity There's a problem with this approach - specifically we need to dismiss the external file share view controller eventually. But more generally, because SN can present a view controller, it's kind success/retry handling is contextual to it's presenting view controller. So, probably the thing to do is duplicate this logic at it's various presentation contexts. // FREEBIE --- .../ConversationView/MessagesViewController.m | 60 +++++-------------- .../SendExternalFileViewController.m | 8 ++- Signal/src/util/ThreadUtil.h | 6 +- Signal/src/util/ThreadUtil.m | 47 ++++++++++++++- 4 files changed, 68 insertions(+), 53 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m index ce78559e0..0b82f9a48 100644 --- a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m +++ b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m @@ -1167,8 +1167,7 @@ typedef enum : NSUInteger { * NO if there were no unconfirmed identities */ - (BOOL)showSafetyNumberConfirmationIfNecessaryWithConfirmationText:(NSString *)confirmationText - completion: - (void (^)(BOOL didConfirmedIdentity))completionHandler + completion:(void (^)(BOOL didConfirmIdentity))completionHandler { return [SafetyNumberConfirmationAlert presentAlertIfNecessaryWithRecipientIds:self.thread.recipientIdentifiers confirmationText:confirmationText @@ -1282,31 +1281,14 @@ typedef enum : NSUInteger { return; } - BOOL didShowSNAlert = - [self showSafetyNumberConfirmationIfNecessaryWithConfirmationText: - NSLocalizedString(@"SAFETY_NUMBER_CHANGED_CONFIRM_SEND_ACTION", - @"button title to confirm sending to a recipient whose " - @"safety number recently changed") - completion:^(BOOL didConfirmIdentity) { - if (didConfirmIdentity) { - [weakSelf didPressSendButton:button - withMessageText:text - senderId:senderId - senderDisplayName:senderDisplayName - date:date - updateKeyboardState:NO]; - } - }]; - if (didShowSNAlert) { - return; - } - text = [text stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceCharacterSet]]; if (text.length > 0) { if ([Environment.preferences soundInForeground]) { [JSQSystemSoundPlayer jsq_playMessageSentSound]; } + + TSOutgoingMessage *_Nullable outgoingMessage; // Limit outgoing text messages to 16kb. // // We convert large text messages to attachments @@ -1317,21 +1299,24 @@ typedef enum : NSUInteger { [SignalAttachment attachmentWithData:[text dataUsingEncoding:NSUTF8StringEncoding] dataUTI:SignalAttachment.kOversizeTextAttachmentUTI filename:nil]; - [self updateLastVisibleTimestamp:[ThreadUtil sendMessageWithAttachment:attachment - inThread:self.thread - messageSender:self.messageSender] - .timestampForSorting]; + outgoingMessage = + [ThreadUtil sendMessageWithAttachment:attachment inThread:self.thread messageSender:self.messageSender]; + if (outgoingMessage != nil) { + [self updateLastVisibleTimestamp:outgoingMessage.timestampForSorting]; + } } else { - [self updateLastVisibleTimestamp:[ThreadUtil sendMessageWithText:text - inThread:self.thread - messageSender:self.messageSender] - .timestampForSorting]; + outgoingMessage = + [ThreadUtil sendMessageWithText:text inThread:self.thread messageSender:self.messageSender]; + if (outgoingMessage != nil) { + [self updateLastVisibleTimestamp:outgoingMessage.timestampForSorting]; + } } self.lastMessageSentDate = [NSDate new]; [self clearUnreadMessagesIndicator]; - if (updateKeyboardState) { + // Don't pop keyboard if we popped the SN alert, else it obscures the SN alert. + if (outgoingMessage != nil && updateKeyboardState) { [self toggleDefaultKeyboard]; } [self clearDraft]; @@ -3703,21 +3688,6 @@ typedef enum : NSUInteger { return; } - BOOL didShowSNAlert = [self - showSafetyNumberConfirmationIfNecessaryWithConfirmationText: - NSLocalizedString(@"SAFETY_NUMBER_CHANGED_CONFIRM_SEND_ACTION", - @"button title to confirm sending to a recipient whose " - @"safety number recently changed") - completion:^(BOOL didConfirmIdentity) { - if (didConfirmIdentity) { - [weakSelf - tryToSendAttachmentIfApproved:attachment]; - } - }]; - if (didShowSNAlert) { - return; - } - if (attachment == nil || [attachment hasError]) { DDLogWarn(@"%@ %s Invalid attachment: %@.", self.tag, diff --git a/Signal/src/ViewControllers/SendExternalFileViewController.m b/Signal/src/ViewControllers/SendExternalFileViewController.m index d075d89cf..691dc9b05 100644 --- a/Signal/src/ViewControllers/SendExternalFileViewController.m +++ b/Signal/src/ViewControllers/SendExternalFileViewController.m @@ -47,9 +47,13 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(self.attachment); OWSAssert(thread); - [ThreadUtil sendMessageWithAttachment:self.attachment inThread:thread messageSender:self.messageSender]; + TSOutgoingMessage *outgoingMessage = + [ThreadUtil sendMessageWithAttachment:self.attachment inThread:thread messageSender:self.messageSender]; - [Environment messageThreadId:thread.uniqueId]; + // Don't pop to thread if we were blocked from creating an outgoing message by untrusted SN. + if (outgoingMessage != nil) { + [Environment messageThreadId:thread.uniqueId]; + } } - (BOOL)canSelectBlockedContact diff --git a/Signal/src/util/ThreadUtil.h b/Signal/src/util/ThreadUtil.h index 8d2e6b16f..c1d8b5126 100644 --- a/Signal/src/util/ThreadUtil.h +++ b/Signal/src/util/ThreadUtil.h @@ -50,9 +50,9 @@ NS_ASSUME_NONNULL_BEGIN inThread:(TSThread *)thread messageSender:(OWSMessageSender *)messageSender; -+ (TSOutgoingMessage *)sendMessageWithAttachment:(SignalAttachment *)attachment - inThread:(TSThread *)thread - messageSender:(OWSMessageSender *)messageSender; ++ (nullable TSOutgoingMessage *)sendMessageWithAttachment:(SignalAttachment *)attachment + inThread:(TSThread *)thread + messageSender:(OWSMessageSender *)messageSender; // This method will create and/or remove any offers and indicators // necessary for this thread. This includes: diff --git a/Signal/src/util/ThreadUtil.m b/Signal/src/util/ThreadUtil.m index 31d1fe507..f81071e4d 100644 --- a/Signal/src/util/ThreadUtil.m +++ b/Signal/src/util/ThreadUtil.m @@ -41,6 +41,26 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(thread); OWSAssert(messageSender); + OWSContactsManager *contactsManager = [Environment getCurrent].contactsManager; + OWSAssert(contactsManager); + + BOOL didShowSNAlert = [SafetyNumberConfirmationAlert + presentAlertIfNecessaryWithRecipientIds:thread.recipientIdentifiers + confirmationText:NSLocalizedString(@"SAFETY_NUMBER_CHANGED_CONFIRM_SEND_ACTION", + @"button title to confirm sending to a recipient whose safety " + @"number recently changed") + contactsManager:contactsManager + completion:^(BOOL didConfirmIdentity) { + if (didConfirmIdentity) { + [self sendMessageWithText:text + inThread:thread + messageSender:messageSender]; + } + }]; + if (didShowSNAlert) { + return nil; + } + OWSDisappearingMessagesConfiguration *configuration = [OWSDisappearingMessagesConfiguration fetchObjectWithUniqueID:thread.uniqueId]; TSOutgoingMessage *message = @@ -61,9 +81,9 @@ NS_ASSUME_NONNULL_BEGIN } -+ (TSOutgoingMessage *)sendMessageWithAttachment:(SignalAttachment *)attachment - inThread:(TSThread *)thread - messageSender:(OWSMessageSender *)messageSender ++ (nullable TSOutgoingMessage *)sendMessageWithAttachment:(SignalAttachment *)attachment + inThread:(TSThread *)thread + messageSender:(OWSMessageSender *)messageSender { OWSAssert([NSThread isMainThread]); OWSAssert(attachment); @@ -72,6 +92,27 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(thread); OWSAssert(messageSender); + OWSContactsManager *contactsManager = [Environment getCurrent].contactsManager; + OWSAssert(contactsManager); + + BOOL didShowSNAlert = [SafetyNumberConfirmationAlert + presentAlertIfNecessaryWithRecipientIds:thread.recipientIdentifiers + confirmationText:NSLocalizedString(@"SAFETY_NUMBER_CHANGED_CONFIRM_SEND_ACTION", + @"button title to confirm sending to a recipient whose safety " + @"number recently changed") + contactsManager:contactsManager + completion:^(BOOL didConfirmIdentity) { + if (didConfirmIdentity) { + [self sendMessageWithAttachment:attachment + inThread:thread + messageSender:messageSender]; + } + }]; + + if (didShowSNAlert) { + return nil; + } + OWSDisappearingMessagesConfiguration *configuration = [OWSDisappearingMessagesConfiguration fetchObjectWithUniqueID:thread.uniqueId]; TSOutgoingMessage *message =