From 81c9dc891958521d1ba3364ea20a38858aff8d6f Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 6 Oct 2020 12:24:06 +1100 Subject: [PATCH 1/3] Fix SSK group leaving race condition --- .../Closed Groups/ClosedGroupsProtocol.swift | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift index b959d17c1..b2aa36302 100644 --- a/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift +++ b/SignalServiceKit/src/Loki/Protocol/Closed Groups/ClosedGroupsProtocol.swift @@ -118,11 +118,18 @@ public final class ClosedGroupsProtocol : NSObject { print("[Loki] Can't remove self and others simultaneously.") return Promise(error: Error.invalidUpdate) } - // Send the update to the group (don't include new ratchets as everyone should regenerate new ratchets individually) - let closedGroupUpdateMessageKind = ClosedGroupUpdateMessage.Kind.info(groupPublicKey: Data(hex: groupPublicKey), name: name, senderKeys: [], - members: membersAsData, admins: adminsAsData) - let closedGroupUpdateMessage = ClosedGroupUpdateMessage(thread: thread, kind: closedGroupUpdateMessageKind) - SSKEnvironment.shared.messageSender.send(closedGroupUpdateMessage, success: { seal.fulfill(()) }, failure: { seal.reject($0) }) + // Establish sessions if needed + establishSessionsIfNeeded(with: [String](members), using: transaction) + // Send the update to the existing members using established channels (don't include new ratchets as everyone should regenerate new ratchets individually) + let promises: [Promise] = oldMembers.map { member in + let thread = TSContactThread.getOrCreateThread(withContactId: member, transaction: transaction) + thread.save(with: transaction) + let closedGroupUpdateMessageKind = ClosedGroupUpdateMessage.Kind.info(groupPublicKey: Data(hex: groupPublicKey), name: name, senderKeys: [], + members: membersAsData, admins: adminsAsData) + let closedGroupUpdateMessage = ClosedGroupUpdateMessage(thread: thread, kind: closedGroupUpdateMessageKind) + return SSKEnvironment.shared.messageSender.sendPromise(message: closedGroupUpdateMessage) + } + when(resolved: promises).done2 { _ in seal.fulfill(()) }.catch2 { seal.reject($0) } promise.done { try! Storage.writeSync { transaction in // Delete all ratchets (it's important that this happens * after * sending out the update) @@ -134,8 +141,6 @@ public final class ClosedGroupsProtocol : NSObject { // Notify the PN server LokiPushNotificationManager.performOperation(.unsubscribe, for: groupPublicKey, publicKey: userPublicKey) } else { - // Establish sessions if needed - establishSessionsIfNeeded(with: [String](members), using: transaction) // Send closed group update messages to any new members using established channels for member in newMembers { let thread = TSContactThread.getOrCreateThread(withContactId: member, transaction: transaction) @@ -203,13 +208,13 @@ public final class ClosedGroupsProtocol : NSObject { return promise } - /// The returned promise is fulfilled when the message has been sent **to the group**. It doesn't wait for the user's new ratchet to be distributed. + /// The returned promise is fulfilled when the group update message has been sent. It doesn't wait for the user's new ratchet to be distributed. @objc(leaveGroupWithPublicKey:transaction:) public static func objc_leave(_ groupPublicKey: String, using transaction: YapDatabaseReadWriteTransaction) -> AnyPromise { return AnyPromise.from(leave(groupPublicKey, using: transaction)) } - /// The returned promise is fulfilled when the message has been sent **to the group**. It doesn't wait for the user's new ratchet to be distributed. + /// The returned promise is fulfilled when the group update message has been sent. It doesn't wait for the user's new ratchet to be distributed. public static func leave(_ groupPublicKey: String, using transaction: YapDatabaseReadWriteTransaction) -> Promise { let userPublicKey = UserDefaults.standard[.masterHexEncodedPublicKey] ?? getUserHexEncodedPublicKey() let groupID = LKGroupUtilities.getEncodedClosedGroupIDAsData(groupPublicKey) From 3c0e5145657f9456837d62f03e1771da0e9d5765 Mon Sep 17 00:00:00 2001 From: nielsandriesse Date: Tue, 6 Oct 2020 14:24:09 +1100 Subject: [PATCH 2/3] Show a loader while the group is updating --- .../View Controllers/EditClosedGroupVC.swift | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Signal/src/Loki/View Controllers/EditClosedGroupVC.swift b/Signal/src/Loki/View Controllers/EditClosedGroupVC.swift index 02548f496..075a146e4 100644 --- a/Signal/src/Loki/View Controllers/EditClosedGroupVC.swift +++ b/Signal/src/Loki/View Controllers/EditClosedGroupVC.swift @@ -241,13 +241,17 @@ final class EditClosedGroupVC : BaseVC, UITableViewDataSource, UITableViewDelega guard members != Set(thread.groupModel.groupMemberIds) || name != thread.groupModel.groupName else { return popToConversationVC(self) } - try! Storage.writeSync { [weak self] transaction in - ClosedGroupsProtocol.update(groupPublicKey, with: members, name: name, transaction: transaction).done(on: DispatchQueue.main) { - guard let self = self else { return } - popToConversationVC(self) - }.catch(on: DispatchQueue.main) { error in - guard let self = self else { return } - self.showError(title: "Couldn't Update Group", message: "Please check your internet connection and try again.") + ModalActivityIndicatorViewController.present(fromViewController: navigationController!, canCancel: false) { [weak self] _ in + try! Storage.writeSync { [weak self] transaction in + ClosedGroupsProtocol.update(groupPublicKey, with: members, name: name, transaction: transaction).done(on: DispatchQueue.main) { + guard let self = self else { return } + self.dismiss(animated: true, completion: nil) // Dismiss the loader + popToConversationVC(self) + }.catch(on: DispatchQueue.main) { error in + guard let self = self else { return } + self.dismiss(animated: true, completion: nil) // Dismiss the loader + self.showError(title: "Couldn't Update Group", message: "Please check your internet connection and try again.") + } } } } From f95d3d171f5f808e828d1bdbd1d7430d487c9c79 Mon Sep 17 00:00:00 2001 From: Niels Andriesse Date: Wed, 7 Oct 2020 15:20:12 +1100 Subject: [PATCH 3/3] Retry PN server requests if needed --- .../LokiPushNotificationManager.swift | 65 ++++++++++--------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/SignalServiceKit/src/Loki/Push Notifications/LokiPushNotificationManager.swift b/SignalServiceKit/src/Loki/Push Notifications/LokiPushNotificationManager.swift index faa163b52..253e63be7 100644 --- a/SignalServiceKit/src/Loki/Push Notifications/LokiPushNotificationManager.swift +++ b/SignalServiceKit/src/Loki/Push Notifications/LokiPushNotificationManager.swift @@ -10,6 +10,7 @@ public final class LokiPushNotificationManager : NSObject { private static let server = "https://live.apns.getsession.org" #endif internal static let pnServerPublicKey = "642a6585919742e5a2d4dc51244964fbcd8bcab2b75612407de58b810740d049" + private static let maxRetryCount: UInt = 4 private static let tokenExpirationInterval: TimeInterval = 12 * 60 * 60 public enum ClosedGroupOperation: String { @@ -28,12 +29,14 @@ public final class LokiPushNotificationManager : NSObject { let url = URL(string: "\(server)/unregister")! let request = TSRequest(url: url, method: "POST", parameters: parameters) request.allHTTPHeaderFields = [ "Content-Type" : "application/json" ] - let promise: Promise = OnionRequestAPI.sendOnionRequest(request, to: server, using: pnServerPublicKey).map2 { response in - guard let json = response["body"] as? JSON else { - return print("[Loki] Couldn't unregister from push notifications.") - } - guard json["code"] as? Int != 0 else { - return print("[Loki] Couldn't unregister from push notifications due to error: \(json["message"] as? String ?? "nil").") + let promise: Promise = attempt(maxRetryCount: maxRetryCount, recoveringOn: DispatchQueue.global()) { + OnionRequestAPI.sendOnionRequest(request, to: server, using: pnServerPublicKey).map2 { response in + guard let json = response["body"] as? JSON else { + return print("[Loki] Couldn't unregister from push notifications.") + } + guard json["code"] as? Int != 0 else { + return print("[Loki] Couldn't unregister from push notifications due to error: \(json["message"] as? String ?? "nil").") + } } } promise.catch2 { error in @@ -68,16 +71,18 @@ public final class LokiPushNotificationManager : NSObject { let url = URL(string: "\(server)/register")! let request = TSRequest(url: url, method: "POST", parameters: parameters) request.allHTTPHeaderFields = [ "Content-Type" : "application/json" ] - let promise: Promise = OnionRequestAPI.sendOnionRequest(request, to: server, using: pnServerPublicKey).map2 { response in - guard let json = response["body"] as? JSON else { - return print("[Loki] Couldn't register device token.") - } - guard json["code"] as? Int != 0 else { - return print("[Loki] Couldn't register device token due to error: \(json["message"] as? String ?? "nil").") + let promise: Promise = attempt(maxRetryCount: maxRetryCount, recoveringOn: DispatchQueue.global()) { + OnionRequestAPI.sendOnionRequest(request, to: server, using: pnServerPublicKey).map2 { response in + guard let json = response["body"] as? JSON else { + return print("[Loki] Couldn't register device token.") + } + guard json["code"] as? Int != 0 else { + return print("[Loki] Couldn't register device token due to error: \(json["message"] as? String ?? "nil").") + } + userDefaults[.deviceToken] = hexEncodedToken + userDefaults[.lastDeviceTokenUpload] = now + userDefaults[.isUsingFullAPNs] = true } - userDefaults[.deviceToken] = hexEncodedToken - userDefaults[.lastDeviceTokenUpload] = now - userDefaults[.isUsingFullAPNs] = true } promise.catch2 { error in print("[Loki] Couldn't register device token.") @@ -103,14 +108,15 @@ public final class LokiPushNotificationManager : NSObject { let url = URL(string: "\(server)/\(operation.rawValue)")! let request = TSRequest(url: url, method: "POST", parameters: parameters) request.allHTTPHeaderFields = [ "Content-Type" : "application/json" ] - let promise = OnionRequestAPI.sendOnionRequest(request, to: server, using: pnServerPublicKey).map2 { response in - guard let json = response["body"] as? JSON else { - return print("[Loki] Couldn't subscribe/unsubscribe closed group: \(closedGroupPublicKey).") + let promise: Promise = attempt(maxRetryCount: maxRetryCount, recoveringOn: DispatchQueue.global()) { + OnionRequestAPI.sendOnionRequest(request, to: server, using: pnServerPublicKey).map2 { response in + guard let json = response["body"] as? JSON else { + return print("[Loki] Couldn't subscribe/unsubscribe closed group: \(closedGroupPublicKey).") + } + guard json["code"] as? Int != 0 else { + return print("[Loki] Couldn't subscribe/unsubscribe for closed group: \(closedGroupPublicKey) due to error: \(json["message"] as? String ?? "nil").") + } } - guard json["code"] as? Int != 0 else { - return print("[Loki] Couldn't subscribe/unsubscribe for closed group: \(closedGroupPublicKey) due to error: \(json["message"] as? String ?? "nil").") - } - return } promise.catch2 { error in print("[Loki] Couldn't subscribe/unsubscribe closed group: \(closedGroupPublicKey).") @@ -124,14 +130,15 @@ public final class LokiPushNotificationManager : NSObject { let url = URL(string: "\(server)/notify")! let request = TSRequest(url: url, method: "POST", parameters: parameters) request.allHTTPHeaderFields = [ "Content-Type" : "application/json" ] - let promise = OnionRequestAPI.sendOnionRequest(request, to: server, using: pnServerPublicKey).map2 { response in - guard let json = response["body"] as? JSON else { - return print("[Loki] Couldn't notify PN server.") - } - guard json["code"] as? Int != 0 else { - return print("[Loki] Couldn't notify PN server due to error: \(json["message"] as? String ?? "nil").") + let promise: Promise = attempt(maxRetryCount: maxRetryCount, recoveringOn: DispatchQueue.global()) { + OnionRequestAPI.sendOnionRequest(request, to: server, using: pnServerPublicKey).map2 { response in + guard let json = response["body"] as? JSON else { + return print("[Loki] Couldn't notify PN server.") + } + guard json["code"] as? Int != 0 else { + return print("[Loki] Couldn't notify PN server due to error: \(json["message"] as? String ?? "nil").") + } } - return } promise.catch2 { error in print("[Loki] Couldn't notify PN server.")