From b206f2dbc9687d94c9ccae428062f6e89ad90531 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 28 Apr 2020 10:21:02 +1000 Subject: [PATCH 1/8] Fix bug in AFR scenario 1 See https://github.com/loki-project/session-protocol-docs/wiki/Auto-Generated-Friend-Requests for more info --- .../src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift index c529ac4c8..7d01bd7e9 100644 --- a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift @@ -183,7 +183,8 @@ public final class SyncMessagesProtocol : NSObject { }) case .requestReceived: thread.saveFriendRequestStatus(.friends, with: transaction) - FriendRequestProtocol.sendFriendRequestAcceptanceMessage(to: hexEncodedPublicKey, in: thread, using: transaction) // TODO: Shouldn't this be acceptFriendRequest so it takes into account multi device? + // Not sendFriendRequestAcceptanceMessage(to:in:using:) to take into account multi device + FriendRequestProtocol.acceptFriendRequest(from: hexEncodedPublicKey, in: thread, using: transaction) default: break } } From 7a21c02edce0d96f15fa97a1eae6b88dd7890346 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 28 Apr 2020 10:21:23 +1000 Subject: [PATCH 2/8] Clean --- .../Loki/Protocol/Multi Device/DeviceLink.swift | 2 +- .../Sync Messages/SyncMessagesProtocol.swift | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Multi Device/DeviceLink.swift b/SignalServiceKit/src/Loki/Protocol/Multi Device/DeviceLink.swift index 7241826a8..eb62f62e1 100644 --- a/SignalServiceKit/src/Loki/Protocol/Multi Device/DeviceLink.swift +++ b/SignalServiceKit/src/Loki/Protocol/Multi Device/DeviceLink.swift @@ -11,7 +11,7 @@ public final class DeviceLink : NSObject, NSCoding { return (userHexEncodedPublicKey == master.hexEncodedPublicKey) ? slave : master } - // MARK: Nested Types + // MARK: Device @objc(LKDevice) public final class Device : NSObject, NSCoding { @objc public let hexEncodedPublicKey: String diff --git a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift index 7d01bd7e9..0d73b8f51 100644 --- a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift @@ -20,15 +20,15 @@ public final class SyncMessagesProtocol : NSObject { // MARK: - Sending @objc(shouldSkipConfigurationSyncMessage) public static func shouldSkipConfigurationSyncMessage() -> Bool { + // FIXME: We added this check to avoid a crash, but we should really figure out why that crash was happening in the first place return !UserDefaults.standard[.hasLaunchedOnce] } @objc(syncContactWithHexEncodedPublicKey:in:) public static func syncContact(_ hexEncodedPublicKey: String, in transaction: YapDatabaseReadTransaction) { - if let thread = TSContactThread.getWithContactId(hexEncodedPublicKey, transaction: transaction) { // TODO: Should this be getOrCreate? - let syncManager = SSKEnvironment.shared.syncManager - syncManager.syncContacts(for: [ SignalAccount(recipientId: hexEncodedPublicKey) ]) - } + guard TSContactThread.getWithContactId(hexEncodedPublicKey, transaction: transaction) != nil else { return } + let syncManager = SSKEnvironment.shared.syncManager + syncManager.syncContacts(for: [ SignalAccount(recipientId: hexEncodedPublicKey) ]) } @objc(syncAllContacts) @@ -46,7 +46,7 @@ public final class SyncMessagesProtocol : NSObject { } friends.append(SignalAccount(recipientId: getUserHexEncodedPublicKey())) // TODO: We sure about this? let syncManager = SSKEnvironment.shared.syncManager - let promises = friends.chunked(by: 3).map { friends -> Promise in + let promises = friends.chunked(by: 3).map { friends -> Promise in // TODO: Does this always fit? return Promise(syncManager.syncContacts(for: friends)).map { _ in } } return when(fulfilled: promises) @@ -60,7 +60,8 @@ public final class SyncMessagesProtocol : NSObject { public static func syncAllClosedGroups() -> Promise { var groups: [TSGroupThread] = [] TSGroupThread.enumerateCollectionObjects { object, _ in - guard let group = object as? TSGroupThread, group.groupModel.groupType == .closedGroup, group.shouldThreadBeVisible, !group.isForceHidden else { return } + guard let group = object as? TSGroupThread, group.groupModel.groupType == .closedGroup, + group.shouldThreadBeVisible, !group.isForceHidden else { return } groups.append(group) } let syncManager = SSKEnvironment.shared.syncManager @@ -81,9 +82,9 @@ public final class SyncMessagesProtocol : NSObject { let messageSender = SSKEnvironment.shared.messageSender messageSender.send(openGroupSyncMessage, success: { seal.fulfill(()) - }) { error in + }, failure: { error in seal.reject(error) - } + }) return promise } From a14d93fa04a61bb7dd970221d926aa15b519c68d Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 28 Apr 2020 11:40:02 +1000 Subject: [PATCH 3/8] Add contact sync message handling test --- ...upParser.swift => ClosedGroupParser.swift} | 2 +- .../Sync Messages/SyncMessagesProtocol.swift | 14 ++++-- .../SyncMessagesProtocolTests.swift | 49 +++++++++++++++++++ 3 files changed, 59 insertions(+), 6 deletions(-) rename SignalServiceKit/src/Loki/Protocol/Sync Messages/{GroupParser.swift => ClosedGroupParser.swift} (95%) create mode 100644 SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocolTests.swift diff --git a/SignalServiceKit/src/Loki/Protocol/Sync Messages/GroupParser.swift b/SignalServiceKit/src/Loki/Protocol/Sync Messages/ClosedGroupParser.swift similarity index 95% rename from SignalServiceKit/src/Loki/Protocol/Sync Messages/GroupParser.swift rename to SignalServiceKit/src/Loki/Protocol/Sync Messages/ClosedGroupParser.swift index 2a42e3d58..3118f569d 100644 --- a/SignalServiceKit/src/Loki/Protocol/Sync Messages/GroupParser.swift +++ b/SignalServiceKit/src/Loki/Protocol/Sync Messages/ClosedGroupParser.swift @@ -1,5 +1,5 @@ -@objc public final class GroupParser : NSObject { +@objc public final class ClosedGroupParser : NSObject { private let data: Data @objc public init(data: Data) { diff --git a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift index 0d73b8f51..45d611418 100644 --- a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift @@ -159,7 +159,11 @@ public final class SyncMessagesProtocol : NSObject { let wasSentByMasterDevice = (masterHexEncodedPublicKey == hexEncodedPublicKey) guard wasSentByMasterDevice, let contacts = syncMessage.contacts, let contactsAsData = contacts.data, contactsAsData.count > 0 else { return } print("[Loki] Contact sync message received.") - let parser = ContactParser(data: contactsAsData) + handleContactSyncMessageData(contactsAsData, using: transaction) + } + + public static func handleContactSyncMessageData(_ data: Data, using transaction: YapDatabaseReadWriteTransaction) { + let parser = ContactParser(data: data) let hexEncodedPublicKeys = parser.parseHexEncodedPublicKeys() // Try to establish sessions for hexEncodedPublicKey in hexEncodedPublicKeys { @@ -199,17 +203,17 @@ public final class SyncMessagesProtocol : NSObject { let wasSentByMasterDevice = (masterHexEncodedPublicKey == hexEncodedPublicKey) guard wasSentByMasterDevice, let groups = syncMessage.groups, let groupsAsData = groups.data, groupsAsData.count > 0 else { return } print("[Loki] Closed group sync message received.") - let parser = GroupParser(data: groupsAsData) + let parser = ClosedGroupParser(data: groupsAsData) let groupModels = parser.parseGroupModels() for groupModel in groupModels { var thread: TSGroupThread! = TSGroupThread(groupId: groupModel.groupId, transaction: transaction) if thread == nil { thread = TSGroupThread.getOrCreateThread(with: groupModel, transaction: transaction) thread.save(with: transaction) - ClosedGroupsProtocol.establishSessionsIfNeeded(with: groupModel.groupMemberIds, in: thread, using: transaction) - let infoMessage = TSInfoMessage(timestamp: NSDate.ows_millisecondTimeStamp(), in: thread, messageType: .typeGroupUpdate, customMessage: "You have joined the group.") - infoMessage.save(with: transaction) } + ClosedGroupsProtocol.establishSessionsIfNeeded(with: groupModel.groupMemberIds, in: thread, using: transaction) + let infoMessage = TSInfoMessage(timestamp: NSDate.ows_millisecondTimeStamp(), in: thread, messageType: .typeGroupUpdate, customMessage: "You have joined the group.") + infoMessage.save(with: transaction) } } diff --git a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocolTests.swift b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocolTests.swift new file mode 100644 index 000000000..281e4b6f9 --- /dev/null +++ b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocolTests.swift @@ -0,0 +1,49 @@ +import PromiseKit +@testable import SignalServiceKit +import XCTest + +class SyncMessagesProtocolTests : XCTestCase { + + private var storage: OWSPrimaryStorage { OWSPrimaryStorage.shared() } + + override func setUp() { + super.setUp() + // Activate the mock environment + ClearCurrentAppContextForTests() + SetCurrentAppContext(TestAppContext()) + MockSSKEnvironment.activate() + // Register a mock user + let identityManager = OWSIdentityManager.shared() + let seed = Randomness.generateRandomBytes(16)! + let keyPair = Curve25519.generateKeyPair(fromSeed: seed + seed) + let databaseConnection = identityManager.value(forKey: "dbConnection") as! YapDatabaseConnection + databaseConnection.setObject(keyPair, forKey: OWSPrimaryStorageIdentityKeyStoreIdentityKey, inCollection: OWSPrimaryStorageIdentityKeyStoreCollection) + TSAccountManager.sharedInstance().phoneNumberAwaitingVerification = keyPair.hexEncodedPublicKey + TSAccountManager.sharedInstance().didRegister() + } + + func testContactSyncMessageHandling() { + // Let's say Alice and Bob have an ongoing conversation. Alice now links a device. Let's call Alice's master device A1 + // and her slave device A2, and let's call Bob's device B. When Alice links A2 to A1, A2 needs to somehow establish a + // session with B (it already established a session with A1 when the devices were linked). How does it do this? + // + // As part of the linking process, A2 should've received a contact sync from A1. Upon receiving this contact sync, + // A2 should send out AFRs to the subset of the contacts it received from A1 for which it doesn't yet have a session (in + // theory this should be all of them). + let base64EncodedContactData = "AAAA7QpCMDU0ZmI2M2IxYTU4YjU1YTcwNjMxODkyOWRjNmQxMWM4ZWY3OTAxMTZhNzRjOWFmNTVmYTZhMzZlNjhmMTYzYTMyEhBZMyAoLi4uOGYxNjNhMzIpIgZvcmFuZ2UqaQpCMDU0ZmI2M2IxYTU4YjU1YTcwNjMxODkyOWRjNmQxMWM4ZWY3OTAxMTZhNzRjOWFmNTVmYTZhMzZlNjhmMTYzYTMyEiEFT7Y7Gli1WnBjGJKdxtEcjveQEWp0ya9V+mo25o8WOjIYADIgXAgtAlrJr81tnuWyk8TgJhdsKzz+yIui5mXnbcMyPk1AAAAAAOwKQjA1Nzg4MmQzM2E4OTI1NDdiOTI2NjIyYjk0ZDZjMWNmYjI1ZmY2YTczZmQ4OTZlMWIxNmY1ODI0NzRjZjQ3MDE2YhIQWTQgKC4uLmNmNDcwMTZiKSIFYnJvd24qaQpCMDU3ODgyZDMzYTg5MjU0N2I5MjY2MjJiOTRkNmMxY2ZiMjVmZjZhNzNmZDg5NmUxYjE2ZjU4MjQ3NGNmNDcwMTZiEiEFeILTOoklR7kmYiuU1sHPsl/2pz/YluGxb1gkdM9HAWsYADIgD1QA1ofVIccRhbx8AnbygQYo5iOiyGUMG/sGNP1ENRJAAAAAAPAKQjA1OTUyYTRiNTFjNDJkZWE2OWEwYWNhNWU2OTgxYTQ2MDk0NGI2Yjc0NjdkOWQ5OTliOWU3NjExNzdkYWI1NzIxMxIQWTEgKC4uLmRhYjU3MjEzKSIJYmx1ZV9ncmV5KmkKQjA1OTUyYTRiNTFjNDJkZWE2OWEwYWNhNWU2OTgxYTQ2MDk0NGI2Yjc0NjdkOWQ5OTliOWU3NjExNzdkYWI1NzIxMxIhBZUqS1HELeppoKyl5pgaRglEtrdGfZ2Zm552EXfatXITGAAyIBkyX0S08IAuov6faUvaxYsfJtdpww1G4LF6bG5vG7L+QAA=" + let contactData = Data(base64Encoded: base64EncodedContactData)! + let parser = ContactParser(data: contactData) + let hexEncodedPublicKeys = parser.parseHexEncodedPublicKeys() + storage.dbReadWriteConnection.readWrite { transaction in + SyncMessagesProtocol.handleContactSyncMessageData(contactData, using: transaction) + } + hexEncodedPublicKeys.forEach { hexEncodedPublicKey in + var thread: TSContactThread! + storage.dbReadWriteConnection.readWrite { transaction in + thread = TSContactThread.getOrCreateThread(withContactId: hexEncodedPublicKey, transaction: transaction) + } + XCTAssert(thread.friendRequestStatus == .requestSent) + } + // TODO: Test the case where Bob has multiple devices + } +} From a9295890c3c51245776072e3f853b07afe82d1b7 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 28 Apr 2020 11:40:31 +1000 Subject: [PATCH 4/8] Update Pods --- Pods | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pods b/Pods index ad77d840d..37d85c157 160000 --- a/Pods +++ b/Pods @@ -1 +1 @@ -Subproject commit ad77d840df0857c6bdecce4aad305137d17a8ef6 +Subproject commit 37d85c1574ddf246b62931bae0843fe3cc07cd64 From fbeea62b326c2fcd478811c0fcabc4b571d8a9b4 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 28 Apr 2020 13:49:03 +1000 Subject: [PATCH 5/8] Fix crash --- .../Sync Messages/SyncMessagesProtocol.swift | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift index 45d611418..ba40872f8 100644 --- a/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Sync Messages/SyncMessagesProtocol.swift @@ -175,16 +175,13 @@ public final class SyncMessagesProtocol : NSObject { let autoGeneratedFRMessage = MultiDeviceProtocol.getAutoGeneratedMultiDeviceFRMessage(for: hexEncodedPublicKey, in: transaction) thread.isForceHidden = true thread.save(with: transaction) + // This takes into account multi device messageSender.send(autoGeneratedFRMessage, success: { - storage.dbReadWriteConnection.readWrite { transaction in - autoGeneratedFRMessage.remove() - thread.isForceHidden = false - } + autoGeneratedFRMessage.remove(with: transaction) + thread.isForceHidden = false }, failure: { error in - storage.dbReadWriteConnection.readWrite { transaction in - autoGeneratedFRMessage.remove() - thread.isForceHidden = false - } + autoGeneratedFRMessage.remove(with: transaction) + thread.isForceHidden = false }) case .requestReceived: thread.saveFriendRequestStatus(.friends, with: transaction) From 37f52b655c3421497d92904d0a53d3cf8eced7da Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 28 Apr 2020 13:49:15 +1000 Subject: [PATCH 6/8] Fix iPhone 8 clipping bug --- Signal/src/Loki/View Controllers/PNModeVC.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Signal/src/Loki/View Controllers/PNModeVC.swift b/Signal/src/Loki/View Controllers/PNModeVC.swift index d0e22f22a..8e3e8a3f0 100644 --- a/Signal/src/Loki/View Controllers/PNModeVC.swift +++ b/Signal/src/Loki/View Controllers/PNModeVC.swift @@ -68,7 +68,8 @@ final class PNModeVC : BaseVC, OptionViewDelegate { // Set up top stack view let topStackView = UIStackView(arrangedSubviews: [ titleLabel, explanationLabel, optionsStackView ]) topStackView.axis = .vertical - topStackView.spacing = isSmallScreen ? Values.smallSpacing : Values.veryLargeSpacing + let isMediumScreen = (UIScreen.main.bounds.height - 667) < 1 + topStackView.spacing = isSmallScreen ? Values.smallSpacing : (isMediumScreen ? Values.mediumSpacing : Values.veryLargeSpacing) topStackView.alignment = .fill // Set up top stack view container let topStackViewContainer = UIView(wrapping: topStackView, withInsets: UIEdgeInsets(top: 0, leading: Values.veryLargeSpacing, bottom: 0, trailing: Values.veryLargeSpacing)) From 632f66f2bac829912c1f913dc3aaef47f23335ff Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 28 Apr 2020 14:14:28 +1000 Subject: [PATCH 7/8] Make device link authorization more reliable --- .../View Controllers/DeviceLinkingModal.swift | 43 ++++++++++--------- .../src/Loki/View Controllers/LandingVC.swift | 2 +- .../translations/en.lproj/Localizable.strings | 2 + .../Multi Device/MultiDeviceProtocol.swift | 2 +- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/Signal/src/Loki/View Controllers/DeviceLinkingModal.swift b/Signal/src/Loki/View Controllers/DeviceLinkingModal.swift index 4865ac877..e3324ae53 100644 --- a/Signal/src/Loki/View Controllers/DeviceLinkingModal.swift +++ b/Signal/src/Loki/View Controllers/DeviceLinkingModal.swift @@ -162,28 +162,30 @@ final class DeviceLinkingModal : Modal, DeviceLinkingSessionDelegate { @objc private func authorizeDeviceLink() { let deviceLink = self.deviceLink! + DeviceLinkingSession.current!.markLinkingRequestAsProcessed() + DeviceLinkingSession.current!.stopListeningForLinkingRequests() let linkingAuthorizationMessage = DeviceLinkingUtilities.getLinkingAuthorizationMessage(for: deviceLink) - ThreadUtil.enqueue(linkingAuthorizationMessage) - SSKEnvironment.shared.messageSender.send(linkingAuthorizationMessage, success: { - let _ = SSKEnvironment.shared.syncManager.syncAllContacts() - let _ = SSKEnvironment.shared.syncManager.syncAllGroups() - let _ = SSKEnvironment.shared.syncManager.syncAllOpenGroups() - let thread = TSContactThread.getOrCreateThread(contactId: deviceLink.slave.hexEncodedPublicKey) - thread.friendRequestStatus = .friends - thread.save() - }) { _ in - print("[Loki] Failed to send device link authorization message.") - } - let session = DeviceLinkingSession.current! - session.stopListeningForLinkingRequests() - session.markLinkingRequestAsProcessed() - dismiss(animated: true, completion: nil) let master = DeviceLink.Device(hexEncodedPublicKey: deviceLink.master.hexEncodedPublicKey, signature: linkingAuthorizationMessage.masterSignature) let signedDeviceLink = DeviceLink(between: master, and: deviceLink.slave) - LokiFileServerAPI.addDeviceLink(signedDeviceLink).done { - self.delegate?.handleDeviceLinkAuthorized(signedDeviceLink) // Intentionally capture self strongly - }.catch { error in + LokiFileServerAPI.addDeviceLink(signedDeviceLink).done { [weak self] in + SSKEnvironment.shared.messageSender.send(linkingAuthorizationMessage, success: { + let thread = TSContactThread.getOrCreateThread(contactId: deviceLink.slave.hexEncodedPublicKey) + thread.save() + let _ = SSKEnvironment.shared.syncManager.syncAllContacts() + let _ = SSKEnvironment.shared.syncManager.syncAllGroups() + let _ = SSKEnvironment.shared.syncManager.syncAllOpenGroups() + thread.friendRequestStatus = .friends + thread.save() + self?.delegate?.handleDeviceLinkAuthorized(signedDeviceLink) // Intentionally capture self strongly + self?.dismiss(animated: true, completion: nil) + }, failure: { error in + print("[Loki] Failed to send device link authorization message.") + let _ = LokiFileServerAPI.removeDeviceLink(signedDeviceLink) // Attempt to roll back + self?.close() + }) + }.catch { [weak self] error in print("[Loki] Failed to add device link due to error: \(error).") + self?.close() // TODO: Show a message to the user } } @@ -214,8 +216,9 @@ final class DeviceLinkingModal : Modal, DeviceLinkingSessionDelegate { session.markLinkingRequestAsProcessed() // Only relevant in master mode delegate?.handleDeviceLinkingModalDismissed() // Only relevant in slave mode if let deviceLink = deviceLink { - OWSPrimaryStorage.shared().dbReadWriteConnection.readWrite { transaction in - OWSPrimaryStorage.shared().removePreKeyBundle(forContact: deviceLink.slave.hexEncodedPublicKey, transaction: transaction) + let storage = OWSPrimaryStorage.shared() + storage.dbReadWriteConnection.readWrite { transaction in + storage.removePreKeyBundle(forContact: deviceLink.slave.hexEncodedPublicKey, transaction: transaction) } } dismiss(animated: true, completion: nil) diff --git a/Signal/src/Loki/View Controllers/LandingVC.swift b/Signal/src/Loki/View Controllers/LandingVC.swift index 3b8eda399..5005becca 100644 --- a/Signal/src/Loki/View Controllers/LandingVC.swift +++ b/Signal/src/Loki/View Controllers/LandingVC.swift @@ -142,7 +142,7 @@ final class LandingVC : BaseVC, LinkDeviceVCDelegate, DeviceLinkingModalDelegate // MARK: Device Linking func requestDeviceLink(with hexEncodedPublicKey: String) { guard ECKeyPair.isValidHexEncodedPublicKey(candidate: hexEncodedPublicKey) else { - let alert = UIAlertController(title: NSLocalizedString("Invalid Public Key", comment: ""), message: NSLocalizedString("Please make sure the public key you entered is correct and try again.", comment: ""), preferredStyle: .alert) + let alert = UIAlertController(title: NSLocalizedString("Invalid Session ID", comment: ""), message: NSLocalizedString("Please make sure the Session ID you entered is correct and try again.", comment: ""), preferredStyle: .alert) alert.addAction(UIAlertAction(title: NSLocalizedString("OK", comment: ""), accessibilityIdentifier: nil, style: .default, handler: nil)) return present(alert, animated: true, completion: nil) } diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index c6ccfc8fc..7bfc47232 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -2823,3 +2823,5 @@ "Confirm" = "Confirm"; "Skip" = "Skip"; "Link Previews" = "Link Previews"; +"Invalid Session ID" = "Invalid Session ID"; +"Please make sure the Session ID you entered is correct and try again." = "Please make sure the Session ID you entered is correct and try again."; diff --git a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift index 183fb8c90..6f9c8d939 100644 --- a/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Multi Device/MultiDeviceProtocol.swift @@ -135,7 +135,7 @@ public final class MultiDeviceProtocol : NSObject { return OWSMessageSend(message: message, thread: thread, recipient: recipient, senderCertificate: senderCertificate, udAccess: recipientUDAccess, localNumber: getUserHexEncodedPublicKey(), success: { - }, failure: { error in + }, failure: { _ in }) } From 2e0c92e842fe89e2f90488d334c1d1cac2354587 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 28 Apr 2020 15:11:26 +1000 Subject: [PATCH 8/8] Fix crash & improve device linking UX --- .../View Controllers/DeviceLinkingModal.swift | 57 ++++++++++++++----- .../translations/en.lproj/Localizable.strings | 4 ++ 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/Signal/src/Loki/View Controllers/DeviceLinkingModal.swift b/Signal/src/Loki/View Controllers/DeviceLinkingModal.swift index e3324ae53..5a712f15c 100644 --- a/Signal/src/Loki/View Controllers/DeviceLinkingModal.swift +++ b/Signal/src/Loki/View Controllers/DeviceLinkingModal.swift @@ -5,6 +5,7 @@ final class DeviceLinkingModal : Modal, DeviceLinkingSessionDelegate { private let mode: Mode private let delegate: DeviceLinkingModalDelegate? private var deviceLink: DeviceLink? + private var hasAuthorizedDeviceLink = false // MARK: Types enum Mode : String { case master, slave } @@ -78,6 +79,13 @@ final class DeviceLinkingModal : Modal, DeviceLinkingSessionDelegate { result.setTitle(NSLocalizedString("Authorize", comment: ""), for: UIControl.State.normal) return result }() + + private lazy var mainStackView: UIStackView = { + let result = UIStackView(arrangedSubviews: [ titleLabel, subtitleLabel, mnemonicLabel, buttonStackView ]) + result.spacing = Values.largeSpacing + result.axis = .vertical + return result + }() // MARK: Lifecycle init(mode: Mode, delegate: DeviceLinkingModalDelegate?) { @@ -107,14 +115,11 @@ final class DeviceLinkingModal : Modal, DeviceLinkingSessionDelegate { } override func populateContentView() { - let stackView = UIStackView(arrangedSubviews: [ titleLabel, subtitleLabel, mnemonicLabel, buttonStackView ]) switch mode { - case .master: stackView.insertArrangedSubview(qrCodeImageViewContainer, at: 0) - case .slave: stackView.insertArrangedSubview(spinner, at: 0) + case .master: mainStackView.insertArrangedSubview(qrCodeImageViewContainer, at: 0) + case .slave: mainStackView.insertArrangedSubview(spinner, at: 0) } - contentView.addSubview(stackView) - stackView.spacing = Values.largeSpacing - stackView.axis = .vertical + contentView.addSubview(mainStackView) switch mode { case .master: let hexEncodedPublicKey = getUserHexEncodedPublicKey() @@ -142,10 +147,10 @@ final class DeviceLinkingModal : Modal, DeviceLinkingSessionDelegate { } authorizeButton.addTarget(self, action: #selector(authorizeDeviceLink), for: UIControl.Event.touchUpInside) authorizeButton.isHidden = true - stackView.pin(.leading, to: .leading, of: contentView, withInset: Values.largeSpacing) - stackView.pin(.top, to: .top, of: contentView, withInset: Values.largeSpacing) - contentView.pin(.trailing, to: .trailing, of: stackView, withInset: Values.largeSpacing) - contentView.pin(.bottom, to: .bottom, of: stackView, withInset: Values.largeSpacing) + mainStackView.pin(.leading, to: .leading, of: contentView, withInset: Values.largeSpacing) + mainStackView.pin(.top, to: .top, of: contentView, withInset: Values.largeSpacing) + contentView.pin(.trailing, to: .trailing, of: mainStackView, withInset: Values.largeSpacing) + contentView.pin(.bottom, to: .bottom, of: mainStackView, withInset: Values.largeSpacing) } // MARK: Device Linking @@ -161,13 +166,23 @@ final class DeviceLinkingModal : Modal, DeviceLinkingSessionDelegate { } @objc private func authorizeDeviceLink() { + guard !hasAuthorizedDeviceLink else { return } + hasAuthorizedDeviceLink = true + mainStackView.removeArrangedSubview(qrCodeImageViewContainer) + mainStackView.insertArrangedSubview(spinner, at: 0) + spinner.set(.height, to: 64) + spinner.startAnimating() + titleLabel.text = NSLocalizedString("Authorizing Device Link", comment: "") + subtitleLabel.text = NSLocalizedString("Please wait while the device link is created. This can take up to a minute.", comment: "") + mnemonicLabel.isHidden = true + buttonStackView.isHidden = true let deviceLink = self.deviceLink! DeviceLinkingSession.current!.markLinkingRequestAsProcessed() DeviceLinkingSession.current!.stopListeningForLinkingRequests() let linkingAuthorizationMessage = DeviceLinkingUtilities.getLinkingAuthorizationMessage(for: deviceLink) let master = DeviceLink.Device(hexEncodedPublicKey: deviceLink.master.hexEncodedPublicKey, signature: linkingAuthorizationMessage.masterSignature) let signedDeviceLink = DeviceLink(between: master, and: deviceLink.slave) - LokiFileServerAPI.addDeviceLink(signedDeviceLink).done { [weak self] in + LokiFileServerAPI.addDeviceLink(signedDeviceLink).done(on: DispatchQueue.main) { [weak self] in SSKEnvironment.shared.messageSender.send(linkingAuthorizationMessage, success: { let thread = TSContactThread.getOrCreateThread(contactId: deviceLink.slave.hexEncodedPublicKey) thread.save() @@ -176,16 +191,28 @@ final class DeviceLinkingModal : Modal, DeviceLinkingSessionDelegate { let _ = SSKEnvironment.shared.syncManager.syncAllOpenGroups() thread.friendRequestStatus = .friends thread.save() - self?.delegate?.handleDeviceLinkAuthorized(signedDeviceLink) // Intentionally capture self strongly - self?.dismiss(animated: true, completion: nil) + DispatchQueue.main.async { + self?.dismiss(animated: true, completion: nil) + self?.delegate?.handleDeviceLinkAuthorized(signedDeviceLink) + } }, failure: { error in print("[Loki] Failed to send device link authorization message.") let _ = LokiFileServerAPI.removeDeviceLink(signedDeviceLink) // Attempt to roll back - self?.close() + DispatchQueue.main.async { + self?.close() + let alert = UIAlertController(title: NSLocalizedString("Device Linking Failed", comment: ""), message: NSLocalizedString("Please check your internet connection and try again", comment: ""), preferredStyle: .alert) + alert.addAction(UIAlertAction(title: NSLocalizedString("OK", comment: ""), style: .default, handler: nil)) + self?.presentingViewController?.present(alert, animated: true, completion: nil) + } }) }.catch { [weak self] error in print("[Loki] Failed to add device link due to error: \(error).") - self?.close() // TODO: Show a message to the user + DispatchQueue.main.async { + self?.close() // TODO: Show a message to the user + let alert = UIAlertController(title: NSLocalizedString("Device Linking Failed", comment: ""), message: NSLocalizedString("Please check your internet connection and try again", comment: ""), preferredStyle: .alert) + alert.addAction(UIAlertAction(title: NSLocalizedString("OK", comment: ""), style: .default, handler: nil)) + self?.presentingViewController?.present(alert, animated: true, completion: nil) + } } } diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index 7bfc47232..0a639289e 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -2825,3 +2825,7 @@ "Link Previews" = "Link Previews"; "Invalid Session ID" = "Invalid Session ID"; "Please make sure the Session ID you entered is correct and try again." = "Please make sure the Session ID you entered is correct and try again."; +"Device Linking Failed" = "Device Linking Failed"; +"Please check your internet connection and try again" = "Please check your internet connection and try again"; +"Authorization Device Link" = "Authorization Device Link"; +"Please wait while the device link is created. This can take up to a minute." = "Please wait while the device link is created. This can take up to a minute.";