From 6e7a0bff5e4f29c03bd00f421643542a692be397 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 10 Feb 2020 10:13:16 -0300 Subject: [PATCH 1/4] Add support for avatars to v2 groups. * Log upload failures. * Finalize group avatar uploads. * Encrypt & decrypt group avatars. --- Signal.xcodeproj/project.pbxproj | 4 + .../groups/GroupV2SnapshotImpl.swift | 7 + .../groups/GroupV2UpdatesImpl.swift | 2 + .../GroupsV2AvatarDownloadOperation.swift | 51 +++ .../groups/GroupsV2ChangeSetImpl.swift | 70 +++- SignalMessaging/groups/GroupsV2Changes.swift | 22 +- SignalMessaging/groups/GroupsV2Impl.swift | 248 +++++++++++- SignalMessaging/groups/GroupsV2Protos.swift | 128 +++++-- SignalMessaging/groups/GroupsV2Test.swift | 108 ++++-- .../groups/StorageService+GroupsV2.swift | 13 + .../profiles/VersionedProfiles.swift | 51 +-- .../src/Contacts/Threads/TSGroupThread.h | 7 - .../src/Contacts/Threads/TSGroupThread.m | 40 -- .../src/Messages/OWSMessageManager.m | 46 ++- .../Stickers/CDNDownloadOperation.swift | 28 +- .../src/Network/API/NetworkManager.swift | 8 + .../src/Network/API/OWSUploadV2.h | 36 +- .../src/Network/API/OWSUploadV2.m | 176 +++------ .../src/Network/API/OWSUploadV2.swift | 107 ++++++ .../Receiving/GroupsV2MessageProcessor.swift | 180 +++++---- SignalServiceKit/src/SignalServiceKit.h | 1 + SignalServiceKit/src/groups/GroupAccess.swift | 5 - .../src/groups/GroupManager.swift | 361 +++++++++++++----- SignalServiceKit/src/groups/GroupsV2.swift | 100 ++++- SignalServiceKit/src/groups/TSGroupModel.h | 3 - SignalServiceKit/src/groups/TSGroupModel.m | 5 - .../src/groups/TSGroupModel.swift | 19 +- .../src/groups/TSGroupModelBuilder.swift | 10 +- 28 files changed, 1286 insertions(+), 550 deletions(-) create mode 100644 SignalMessaging/groups/GroupsV2AvatarDownloadOperation.swift create mode 100644 SignalServiceKit/src/Network/API/OWSUploadV2.swift diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index bb3551b060..dc867a5f83 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -173,6 +173,7 @@ 3470249E2385B6360078D72C /* OWSProfileManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3470249D2385B6360078D72C /* OWSProfileManager.swift */; }; 347024A0238C85850078D72C /* VersionedProfiles.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3470249F238C85850078D72C /* VersionedProfiles.swift */; }; 347137FC22A1BCE800F43A63 /* OWSMessageViewOnceView.m in Sources */ = {isa = PBXBuildFile; fileRef = 347137FB22A1BCE800F43A63 /* OWSMessageViewOnceView.m */; }; + 347191F923F457BD003A3106 /* GroupsV2AvatarDownloadOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 347191F823F457BD003A3106 /* GroupsV2AvatarDownloadOperation.swift */; }; 347850311FD7494A007B8332 /* dripicons-v2.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 34330A5B1E787A9800DF2FB9 /* dripicons-v2.ttf */; }; 347850321FD7494A007B8332 /* ElegantIcons.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 34330A5D1E787BD800DF2FB9 /* ElegantIcons.ttf */; }; 347850331FD7494A007B8332 /* fontawesome-webfont.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 34330A591E7875FB00DF2FB9 /* fontawesome-webfont.ttf */; }; @@ -1027,6 +1028,7 @@ 3470249F238C85850078D72C /* VersionedProfiles.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VersionedProfiles.swift; sourceTree = ""; }; 347137FA22A1BCE800F43A63 /* OWSMessageViewOnceView.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSMessageViewOnceView.h; sourceTree = ""; }; 347137FB22A1BCE800F43A63 /* OWSMessageViewOnceView.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OWSMessageViewOnceView.m; sourceTree = ""; }; + 347191F823F457BD003A3106 /* GroupsV2AvatarDownloadOperation.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GroupsV2AvatarDownloadOperation.swift; sourceTree = ""; }; 34784592237C4813000D1CAA /* hi */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = hi; path = translations/hi.lproj/Localizable.strings; sourceTree = ""; }; 34784593237C4819000D1CAA /* ms */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ms; path = translations/ms.lproj/Localizable.strings; sourceTree = ""; }; 347850561FD86544007B8332 /* SAEFailedViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SAEFailedViewController.swift; sourceTree = ""; }; @@ -2345,6 +2347,7 @@ 34BB3C5723C6644B001651FC /* groups */ = { isa = PBXGroup; children = ( + 347191F823F457BD003A3106 /* GroupsV2AvatarDownloadOperation.swift */, 34F0566923DA209300265283 /* GroupsV2Changes.swift */, 34BB3C5923C6644B001651FC /* GroupsV2ChangeSetImpl.swift */, 34BB3C5C23C6644B001651FC /* GroupsV2Impl.swift */, @@ -4371,6 +4374,7 @@ 451F8A441FD7156B005CB9DA /* BlockListUIUtils.m in Sources */, 34AC0A1E211B39EA00997B47 /* ThreadViewHelper.m in Sources */, 34BBC85A220C7ADA00857249 /* ImageEditorTextItem.swift in Sources */, + 347191F923F457BD003A3106 /* GroupsV2AvatarDownloadOperation.swift in Sources */, 34641E182088D7E900E2EDE5 /* OWSScreenLock.swift in Sources */, 344DC9AF226E483D004E7322 /* ManageStickersViewController.swift in Sources */, 349EA07C2162AEA800F7B17F /* OWS111UDAttributesMigration.swift in Sources */, diff --git a/SignalMessaging/groups/GroupV2SnapshotImpl.swift b/SignalMessaging/groups/GroupV2SnapshotImpl.swift index 632176a940..9048408dd2 100644 --- a/SignalMessaging/groups/GroupV2SnapshotImpl.swift +++ b/SignalMessaging/groups/GroupV2SnapshotImpl.swift @@ -40,6 +40,9 @@ public struct GroupV2SnapshotImpl: GroupV2Snapshot { public let title: String + public let avatarUrlPath: String? + public let avatarData: Data? + private let members: [Member] private let pendingMembers: [PendingMember] @@ -58,6 +61,8 @@ public struct GroupV2SnapshotImpl: GroupV2Snapshot { groupProto: GroupsProtoGroup, revision: UInt32, title: String, + avatarUrlPath: String?, + avatarData: Data?, members: [Member], pendingMembers: [PendingMember], accessControlForAttributes: GroupsProtoAccessControlAccessRequired, @@ -69,6 +74,8 @@ public struct GroupV2SnapshotImpl: GroupV2Snapshot { self.groupProto = groupProto self.revision = revision self.title = title + self.avatarUrlPath = avatarUrlPath + self.avatarData = avatarData self.members = members self.pendingMembers = pendingMembers self.accessControlForAttributes = accessControlForAttributes diff --git a/SignalMessaging/groups/GroupV2UpdatesImpl.swift b/SignalMessaging/groups/GroupV2UpdatesImpl.swift index 5aced01a89..8705ab8eea 100644 --- a/SignalMessaging/groups/GroupV2UpdatesImpl.swift +++ b/SignalMessaging/groups/GroupV2UpdatesImpl.swift @@ -339,6 +339,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { public func updateGroupWithChangeActions(groupId: Data, changeActionsProto: GroupsProtoGroupChangeActions, + downloadedAvatars: GroupV2DownloadedAvatars, transaction: SDSAnyWriteTransaction) throws -> TSGroupThread { // GroupsV2 TODO: When applying snapshots and change actions to the local // database, we should check revision in the local database. @@ -355,6 +356,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift { } let changedGroupModel = try GroupsV2Changes.applyChangesToGroupModel(groupThread: groupThread, changeActionsProto: changeActionsProto, + downloadedAvatars: downloadedAvatars, transaction: transaction) guard changedGroupModel.newGroupModel.groupV2Revision > changedGroupModel.oldGroupModel.groupV2Revision else { throw OWSAssertionError("Invalid groupV2Revision: \(changedGroupModel.newGroupModel.groupV2Revision).") diff --git a/SignalMessaging/groups/GroupsV2AvatarDownloadOperation.swift b/SignalMessaging/groups/GroupsV2AvatarDownloadOperation.swift new file mode 100644 index 0000000000..534fae3b1c --- /dev/null +++ b/SignalMessaging/groups/GroupsV2AvatarDownloadOperation.swift @@ -0,0 +1,51 @@ +// +// Copyright (c) 2020 Open Whisper Systems. All rights reserved. +// + +import Foundation +import PromiseKit +import SignalServiceKit + +class GroupsV2AvatarDownloadOperation: CDNDownloadOperation { + + private let urlPath: String + private let maxDownloadSize: UInt? + public let promise: Promise + private let resolver: Resolver + + public required init(urlPath: String, + maxDownloadSize: UInt? = nil) { + self.urlPath = urlPath + self.maxDownloadSize = maxDownloadSize + + let (promise, resolver) = Promise.pending() + self.promise = promise + self.resolver = resolver + + super.init() + } + + override public func run() { + firstly { + return try tryToDownload(urlPath: urlPath, maxDownloadSize: maxDownloadSize) + }.done(on: DispatchQueue.global()) { [weak self] data in + guard let self = self else { + return + } + + self.resolver.fulfill(data) + self.reportSuccess() + }.catch(on: DispatchQueue.global()) { [weak self] error in + guard let self = self else { + return + } + return self.reportError(withUndefinedRetry: error) + }.retainUntilComplete() + } + + override public func didFail(error: Error) { + Logger.error("Download exhausted retries: \(error)") + + resolver.reject(error) + } +} diff --git a/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift b/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift index e49b262407..45ef0ee424 100644 --- a/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift +++ b/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift @@ -60,6 +60,10 @@ public class GroupsV2ChangeSetImpl: NSObject, GroupsV2ChangeSet { // When clearing the title, this will be the empty string. private var title: String? + public var newAvatarData: Data? + public var newAvatarUrlPath: String? + private var shouldUpdateAvatar = false + private var membersToAdd = [UUID: GroupsProtoMemberRole]() private var membersToRemove = [UUID]() private var membersToChangeRole = [UUID: GroupsProtoMemberRole]() @@ -107,9 +111,11 @@ public class GroupsV2ChangeSetImpl: NSObject, GroupsV2ChangeSet { oldDMConfiguration: OWSDisappearingMessagesConfiguration, newDMConfiguration: OWSDisappearingMessagesConfiguration, transaction: SDSAnyReadTransaction) throws { - - if oldGroupModel.groupName != newGroupModel.groupName { - setTitle(newGroupModel.groupName) + guard let oldGroupModel = oldGroupModel as? TSGroupModelV2 else { + throw OWSAssertionError("Invalid oldGroupModel.") + } + guard let newGroupModel = newGroupModel as? TSGroupModelV2 else { + throw OWSAssertionError("Invalid newGroupModel.") } guard groupId == oldGroupModel.groupId else { throw OWSAssertionError("Mismatched groupId.") @@ -118,6 +124,23 @@ public class GroupsV2ChangeSetImpl: NSObject, GroupsV2ChangeSet { throw OWSAssertionError("Mismatched groupId.") } + // GroupsV2 TODO: Will production implementation of encryptString() pad? + let newTitle = newGroupModel.groupName?.stripped ?? " " + if oldGroupModel.groupName != newTitle { + setTitle(newTitle) + } + + if oldGroupModel.groupAvatarData != newGroupModel.groupAvatarData { + let hasAvatarUrlPath = newGroupModel.groupAvatarUrlPath != nil + let hasAvatarData = newGroupModel.groupAvatarData != nil + guard hasAvatarUrlPath == hasAvatarData else { + throw OWSAssertionError("hasAvatarUrlPath: \(hasAvatarData) != hasAvatarData.") + } + + setAvatar(avatarData: newGroupModel.groupAvatarData, + avatarUrlPath: newGroupModel.groupAvatarUrlPath) + } + let oldGroupMembership = oldGroupModel.groupMembership let newGroupMembership = newGroupModel.groupMembership @@ -179,8 +202,6 @@ public class GroupsV2ChangeSetImpl: NSObject, GroupsV2ChangeSet { if oldDisappearingMessageToken != newDisappearingMessageToken { setNewDisappearingMessageToken(newDisappearingMessageToken) } - - // GroupsV2 TODO: Calculate other changed state, e.g. avatar. } @objc @@ -190,6 +211,17 @@ public class GroupsV2ChangeSetImpl: NSObject, GroupsV2ChangeSet { self.title = value ?? "" } + @objc + public func setAvatar(avatarData: Data?, avatarUrlPath: String?) { + assert(self.newAvatarData == nil) + assert(self.newAvatarUrlPath == nil) + assert(!self.shouldUpdateAvatar) + + self.newAvatarData = avatarData + self.newAvatarUrlPath = avatarUrlPath + self.shouldUpdateAvatar = true + } + @objc public func addNormalMember(_ uuid: UUID) { addMember(uuid, role: .default) @@ -315,6 +347,9 @@ public class GroupsV2ChangeSetImpl: NSObject, GroupsV2ChangeSet { private func buildGroupChangeProto(currentGroupModel: TSGroupModel, currentDisappearingMessageToken: DisappearingMessageToken, profileKeyCredentialMap: ProfileKeyCredentialMap) throws -> GroupsProtoGroupChangeActions { + guard let currentGroupModel = currentGroupModel as? TSGroupModelV2 else { + throw OWSAssertionError("Invalid currentGroupModel.") + } let groupV2Params = try GroupV2Params(groupModel: currentGroupModel) let actionsBuilder = GroupsProtoGroupChangeActions.builder() @@ -339,6 +374,18 @@ public class GroupsV2ChangeSetImpl: NSObject, GroupsV2ChangeSet { didChange = true } + if shouldUpdateAvatar, + newAvatarUrlPath != currentGroupModel.groupAvatarUrlPath { + let actionBuilder = GroupsProtoGroupChangeActionsModifyAvatarAction.builder() + if let avatarUrlPath = newAvatarUrlPath { + actionBuilder.setAvatar(avatarUrlPath) + } else { + // We're clearing the avatar. + } + actionsBuilder.setModifyAvatar(try actionBuilder.build()) + didChange = true + } + let currentGroupMembership = currentGroupModel.groupMembership for (uuid, role) in self.membersToAdd { guard !currentGroupMembership.contains(uuid) else { @@ -498,17 +545,4 @@ public class GroupsV2ChangeSetImpl: NSObject, GroupsV2ChangeSet { return try actionsBuilder.build() } - - // GroupsV2 TODO: Ensure that we are correctly building all of the following actions: - // - // NOTE: This should be used after you rotate your profile key. - // This presumably needs to be done in a durable way. - // message ModifyMemberProfileKeyAction { - // bytes presentation = 1; - // } - // - // NOTE: This won't be easy. - // message ModifyAvatarAction { - // string avatar = 1; - // } } diff --git a/SignalMessaging/groups/GroupsV2Changes.swift b/SignalMessaging/groups/GroupsV2Changes.swift index 224c97d284..4fbfd91f48 100644 --- a/SignalMessaging/groups/GroupsV2Changes.swift +++ b/SignalMessaging/groups/GroupsV2Changes.swift @@ -49,6 +49,7 @@ public class GroupsV2Changes { // exactly 1 higher. class func applyChangesToGroupModel(groupThread: TSGroupThread, changeActionsProto: GroupsProtoGroupChangeActions, + downloadedAvatars: GroupV2DownloadedAvatars, transaction: SDSAnyReadTransaction) throws -> ChangedGroupModel { let oldGroupModel = groupThread.groupModel let groupsVersion = oldGroupModel.groupsVersion @@ -75,6 +76,12 @@ public class GroupsV2Changes { } var newGroupName: String? = oldGroupModel.groupName + var newAvatarData: Data? = oldGroupModel.groupAvatarData + var newAvatarUrlPath: String? + if let oldGroupModelV2 = oldGroupModel as? TSGroupModelV2 { + newAvatarUrlPath = oldGroupModelV2.groupAvatarUrlPath + } + let oldGroupMembership = oldGroupModel.groupMembership var groupMembershipBuilder = oldGroupMembership.asBuilder @@ -241,14 +248,21 @@ public class GroupsV2Changes { if let titleData = action.title { newGroupName = try groupV2Params.decryptString(titleData) } else { - // Other client cleared the group title. + // Change clears the group title. newGroupName = nil } } if let action = changeActionsProto.modifyAvatar { - // GroupsV2 TODO: Handle avatars. - // GroupsProtoGroupChangeActionsModifyAvatarAction + if let avatarUrl = action.avatar, + !avatarUrl.isEmpty { + newAvatarData = try downloadedAvatars.avatarData(for: avatarUrl) + newAvatarUrlPath = avatarUrl + } else { + // Change clears the group avatar. + newAvatarData = nil + newAvatarUrlPath = nil + } } var newDisappearingMessageToken: DisappearingMessageToken? @@ -280,8 +294,6 @@ public class GroupsV2Changes { let newGroupMembership = groupMembershipBuilder.build() let newGroupAccess = GroupAccess(members: newMembersAccess, attributes: newAttributesAccess) - // GroupsV2 TODO: Avatar. - let newAvatarData: Data? = oldGroupModel.groupAvatarData var builder = oldGroupModel.asBuilder builder.name = newGroupName diff --git a/SignalMessaging/groups/GroupsV2Impl.swift b/SignalMessaging/groups/GroupsV2Impl.swift index 71dbe731ba..27065aeec2 100644 --- a/SignalMessaging/groups/GroupsV2Impl.swift +++ b/SignalMessaging/groups/GroupsV2Impl.swift @@ -37,8 +37,8 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return OWSProfileManager.shared() } - private var groupV2Updates: GroupV2Updates { - return SSKEnvironment.shared.groupV2Updates + private var groupV2Updates: GroupV2UpdatesSwift { + return SSKEnvironment.shared.groupV2Updates as! GroupV2UpdatesSwift } // MARK: - @@ -106,7 +106,7 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { } } - return self.performServiceRequest(requestBuilder: requestBuilder).asVoid() + return performServiceRequest(requestBuilder: requestBuilder).asVoid() } // MARK: - Update Group @@ -169,7 +169,7 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return firstly { return self.performServiceRequest(requestBuilder: requestBuilder) - }.map(on: DispatchQueue.global()) { (response: ServiceResponse) -> UpdatedV2Group in + }.then(on: DispatchQueue.global()) { (response: ServiceResponse) -> Promise in guard let changeActionsProtoData = response.responseObject as? Data else { throw OWSAssertionError("Invalid responseObject.") @@ -177,13 +177,21 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { let changeActionsProto = try GroupsV2Protos.parseAndVerifyChangeActionsProto(changeActionsProtoData, ignoreSignature: true) + // Collect avatar state from our change set so that we can + // avoid downloading any avatars we just uploaded while + // applying the change set locally. + let downloadedAvatars = GroupV2DownloadedAvatars.from(changeSet: changeSet) + // GroupsV2 TODO: Instead of loading the group model from the database, // we should use exactly the same group model that was used to construct // the update request - which should reflect pre-update service state. - let updatedGroupThread = try self.databaseStorage.write { transaction throws -> TSGroupThread in - return try self.groupV2Updates.updateGroupWithChangeActions(groupId: groupId, - changeActionsProto: changeActionsProto, - transaction: transaction) + return firstly { + return self.updateGroupWithChangeActions(groupId: groupId, + changeActionsProto: changeActionsProto, + justUploadedAvatars: downloadedAvatars, + groupV2Params: groupV2Params) + }.map(on: DispatchQueue.global()) { (groupThread: TSGroupThread) -> UpdatedV2Group in + return UpdatedV2Group(groupThread: groupThread, changeActionsProtoData: changeActionsProtoData) } // GroupsV2 TODO: Propagate failure in a consumable way. @@ -205,8 +213,6 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { {encoded_current_group_record} */ - - return UpdatedV2Group(groupThread: updatedGroupThread, changeActionsProtoData: changeActionsProtoData) }.then(on: DispatchQueue.global()) { (updatedV2Group: UpdatedV2Group) -> Promise in GroupManager.updateProfileWhitelist(withGroupThread: updatedV2Group.groupThread) @@ -223,6 +229,80 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { } } + public func updateGroupWithChangeActions(groupId: Data, + changeActionsProto: GroupsProtoGroupChangeActions, + groupSecretParamsData: Data) throws -> Promise { + let groupV2Params = try GroupV2Params(groupSecretParamsData: groupSecretParamsData) + return updateGroupWithChangeActions(groupId: groupId, + changeActionsProto: changeActionsProto, + justUploadedAvatars: nil, + groupV2Params: groupV2Params) + } + + public func updateGroupWithChangeActions(groupId: Data, + changeActionsProto: GroupsProtoGroupChangeActions, + groupV2Params: GroupV2Params) -> Promise { + return updateGroupWithChangeActions(groupId: groupId, + changeActionsProto: changeActionsProto, + justUploadedAvatars: nil, + groupV2Params: groupV2Params) + } + + private func updateGroupWithChangeActions(groupId: Data, + changeActionsProto: GroupsProtoGroupChangeActions, + justUploadedAvatars: GroupV2DownloadedAvatars?, + groupV2Params: GroupV2Params) -> Promise { + + return firstly { + self.fetchAllAvatarData(changeActionsProto: changeActionsProto, + justUploadedAvatars: justUploadedAvatars, + groupV2Params: groupV2Params) + }.map(on: DispatchQueue.global()) { (downloadedAvatars: GroupV2DownloadedAvatars) -> TSGroupThread in + return try self.databaseStorage.write { transaction in + return try self.groupV2Updates.updateGroupWithChangeActions(groupId: groupId, + changeActionsProto: changeActionsProto, + downloadedAvatars: downloadedAvatars, + transaction: transaction) + } + } + } + + // MARK: - Upload Avatar + + public func uploadGroupAvatar(avatarData: Data, + groupSecretParamsData: Data) -> Promise { + return firstly { () -> Promise in + let groupV2Params = try GroupV2Params(groupSecretParamsData: groupSecretParamsData) + return self.uploadGroupAvatar(avatarData: avatarData, groupV2Params: groupV2Params) + } + } + + private func uploadGroupAvatar(avatarData: Data, + groupV2Params: GroupV2Params) -> Promise { + let requestBuilder: RequestBuilder = { (authCredential, sessionManager) in + return DispatchQueue.global().async(.promise) { () -> NSURLRequest in + return try StorageService.buildGroupAvatarUploadFormRequest(groupV2Params: groupV2Params, + sessionManager: sessionManager, + authCredential: authCredential) + } + } + + return firstly { + return self.performServiceRequest(requestBuilder: requestBuilder) + }.map(on: DispatchQueue.global()) { (response: ServiceResponse) -> GroupsProtoAvatarUploadAttributes in + + guard let protoData = response.responseObject as? Data else { + throw OWSAssertionError("Invalid responseObject.") + } + return try GroupsProtoAvatarUploadAttributes.parseData(protoData) + }.map(on: DispatchQueue.global()) { (avatarUploadAttributes: GroupsProtoAvatarUploadAttributes) throws -> OWSUploadForm in + try OWSUploadForm.parse(proto: avatarUploadAttributes) + }.then(on: DispatchQueue.global()) { (uploadForm: OWSUploadForm) -> Promise in + let encryptedData = try groupV2Params.encryptBlob(avatarData) + return OWSUploadV2.upload(data: encryptedData, uploadForm: uploadForm, uploadUrlPath: "") + } + } + // MARK: - Fetch Current Group State public func fetchCurrentGroupV2Snapshot(groupModel: TSGroupModel) -> Promise { @@ -233,10 +313,24 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return Promise(error: OWSAssertionError("Missing groupSecretParamsData.")) } - return self.fetchCurrentGroupV2Snapshot(groupSecretParamsData: groupSecretParamsData) + // Collect the avatar state to avoid an unnecessary download in the + // case where we've just created this group but not yet inserted it + // into the database. + var justUploadedAvatars: GroupV2DownloadedAvatars? + if let groupModel = groupModel as? TSGroupModelV2 { + justUploadedAvatars = GroupV2DownloadedAvatars.from(groupModel: groupModel) + } + return self.fetchCurrentGroupV2Snapshot(groupSecretParamsData: groupSecretParamsData, + justUploadedAvatars: justUploadedAvatars) } public func fetchCurrentGroupV2Snapshot(groupSecretParamsData: Data) -> Promise { + return fetchCurrentGroupV2Snapshot(groupSecretParamsData: groupSecretParamsData, + justUploadedAvatars: nil) + } + + private func fetchCurrentGroupV2Snapshot(groupSecretParamsData: Data, + justUploadedAvatars: GroupV2DownloadedAvatars?) -> Promise { guard let localUuid = tsAccountManager.localUuid else { return Promise(error: OWSAssertionError("Missing localUuid.")) } @@ -244,12 +338,14 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return try GroupV2Params(groupSecretParamsData: groupSecretParamsData) }.then(on: DispatchQueue.global()) { (groupV2Params: GroupV2Params) -> Promise in return self.fetchCurrentGroupV2Snapshot(groupV2Params: groupV2Params, - localUuid: localUuid) + localUuid: localUuid, + justUploadedAvatars: justUploadedAvatars) } } private func fetchCurrentGroupV2Snapshot(groupV2Params: GroupV2Params, - localUuid: UUID) -> Promise { + localUuid: UUID, + justUploadedAvatars: GroupV2DownloadedAvatars?) -> Promise { let requestBuilder: RequestBuilder = { (authCredential, sessionManager) in return DispatchQueue.global().async(.promise) { () -> NSURLRequest in return try StorageService.buildFetchCurrentGroupV2SnapshotRequest(groupV2Params: groupV2Params, @@ -260,12 +356,19 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return firstly { () -> Promise in return self.performServiceRequest(requestBuilder: requestBuilder) - }.map(on: DispatchQueue.global()) { (response: ServiceResponse) -> GroupV2Snapshot in + }.map(on: DispatchQueue.global()) { (response: ServiceResponse) -> GroupsProtoGroup in guard let groupProtoData = response.responseObject as? Data else { throw OWSAssertionError("Invalid responseObject.") } - let groupProto = try GroupsProtoGroup.parseData(groupProtoData) - return try GroupsV2Protos.parse(groupProto: groupProto, groupV2Params: groupV2Params) + return try GroupsProtoGroup.parseData(groupProtoData) + }.then(on: DispatchQueue.global()) { (groupProto: GroupsProtoGroup) -> Promise<(GroupsProtoGroup, GroupV2DownloadedAvatars)> in + return firstly { + self.fetchAllAvatarData(groupProto: groupProto, justUploadedAvatars: justUploadedAvatars, groupV2Params: groupV2Params) + }.map(on: DispatchQueue.global()) { (downloadedAvatars: GroupV2DownloadedAvatars) -> (GroupsProtoGroup, GroupV2DownloadedAvatars) in + return (groupProto, downloadedAvatars) + } + }.map(on: DispatchQueue.global()) { (groupProto: GroupsProtoGroup, downloadedAvatars: GroupV2DownloadedAvatars) -> GroupV2Snapshot in + return try GroupsV2Protos.parse(groupProto: groupProto, downloadedAvatars: downloadedAvatars, groupV2Params: groupV2Params) } } @@ -319,12 +422,113 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return firstly { () -> Promise in return self.performServiceRequest(requestBuilder: requestBuilder) - }.map(on: DispatchQueue.global()) { (response: ServiceResponse) -> [GroupV2Change] in + }.map(on: DispatchQueue.global()) { (response: ServiceResponse) -> GroupsProtoGroupChanges in guard let groupChangesProtoData = response.responseObject as? Data else { throw OWSAssertionError("Invalid responseObject.") } - let groupChangesProto = try GroupsProtoGroupChanges.parseData(groupChangesProtoData) - return try GroupsV2Protos.parse(groupChangesProto: groupChangesProto, groupV2Params: groupV2Params) + return try GroupsProtoGroupChanges.parseData(groupChangesProtoData) + }.then(on: DispatchQueue.global()) { (groupChangesProto: GroupsProtoGroupChanges) -> Promise<[GroupV2Change]> in + return firstly { + self.fetchAllAvatarData(groupChangesProto: groupChangesProto, groupV2Params: groupV2Params) + }.map(on: DispatchQueue.global()) { (downloadedAvatars: GroupV2DownloadedAvatars) -> [GroupV2Change] in + try GroupsV2Protos.parse(groupChangesProto: groupChangesProto, + downloadedAvatars: downloadedAvatars, + groupV2Params: groupV2Params) + } + } + } + + // MARK: - Avatar Downloads + + // Before we can apply snapshots/changes from the service, we + // need to download all avatars they use. We can skip downloads + // in a couple of cases: + // + // * We just created the group. + // * We just updated the group and we're applying those changes. + private func fetchAllAvatarData(groupProto: GroupsProtoGroup? = nil, + groupChangesProto: GroupsProtoGroupChanges? = nil, + changeActionsProto: GroupsProtoGroupChangeActions? = nil, + justUploadedAvatars: GroupV2DownloadedAvatars? = nil, + groupV2Params: GroupV2Params) -> Promise { + + var downloadedAvatars = GroupV2DownloadedAvatars() + + // Creating or updating a group is a multi-step process + // that can involve uploading an avatar, updating the + // group on the service, then updating the local database. + // We can skip downloading an avatar that we just uploaded + // using justUploadedAvatars. + if let justUploadedAvatars = justUploadedAvatars { + downloadedAvatars.merge(justUploadedAvatars) + } + + return DispatchQueue.global().async(.promise) { () throws -> Void in + // First step - try to skip downloading the current group avatar. + let groupId = try self.groupId(forGroupSecretParamsData: groupV2Params.groupSecretParamsData) + guard let groupThread = (self.databaseStorage.read { transaction in + return TSGroupThread.fetch(groupId: groupId, transaction: transaction) + }) else { + // Thread doesn't exist in database yet. + return () + } + guard let groupModel = groupThread.groupModel as? TSGroupModelV2 else { + throw OWSAssertionError("Invalid groupModel.") + } + // Try to add avatar from group model, if any. + downloadedAvatars.merge(GroupV2DownloadedAvatars.from(groupModel: groupModel)) + return () + }.then(on: DispatchQueue.global()) { () -> Promise<[String]> in + GroupsV2Protos.collectAvatarUrlPaths(groupProto: groupProto, + groupChangesProto: groupChangesProto, + changeActionsProto: changeActionsProto, + groupV2Params: groupV2Params) + }.then(on: DispatchQueue.global()) { (protoAvatarUrlPaths: [String]) -> Promise in + + let undownloadedAvatarUrlPaths = Set(protoAvatarUrlPaths).subtracting(downloadedAvatars.avatarUrlPaths) + guard !undownloadedAvatarUrlPaths.isEmpty else { + return Promise.value(downloadedAvatars) + } + + // We need to "populate" any group changes that have a + // avatar with the avatar data. + var promises = [Promise<(String, Data)>]() + for avatarUrlPath in undownloadedAvatarUrlPaths { + let promise = firstly { + self.fetchAvatarData(avatarUrlPath: avatarUrlPath, + groupV2Params: groupV2Params) + }.map(on: DispatchQueue.global()) { (avatarData: Data) -> (String, Data) in + return (avatarUrlPath, avatarData) + } + promises.append(promise) + } + return firstly { + when(fulfilled: promises) + }.map(on: DispatchQueue.global()) { (avatars: [(String, Data)]) -> GroupV2DownloadedAvatars in + for (avatarUrlPath, avatarData) in avatars { + downloadedAvatars.set(avatarData: avatarData, avatarUrlPath: avatarUrlPath) + } + return downloadedAvatars + } + } + } + + let avatarDownloadQueue: OperationQueue = { + let operationQueue = OperationQueue() + operationQueue.name = "avatarDownloadQueue" + operationQueue.maxConcurrentOperationCount = 3 + return operationQueue + }() + + private func fetchAvatarData(avatarUrlPath: String, + groupV2Params: GroupV2Params) -> Promise { + let operation = GroupsV2AvatarDownloadOperation(urlPath: avatarUrlPath) + let promise = operation.promise + avatarDownloadQueue.addOperation(operation) + return firstly { + return promise + }.map(on: DispatchQueue.global()) { (avatarData: Data) -> Data in + return try groupV2Params.decryptBlob(avatarData) } } @@ -488,6 +692,9 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { } owsFailDebug("Response error: \(error)") + #if TESTABLE_BUILD + TSNetworkManager.logCurl(for: blockTask) + #endif return resolver.reject(error) } @@ -496,6 +703,9 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { Logger.info("Request succeeded: \(String(describing: request.httpMethod)) \(String(describing: request.url))") case 401: Logger.warn("Request not authorized.") + #if TESTABLE_BUILD + TSNetworkManager.logCurl(for: blockTask) + #endif return resolver.reject(GroupsV2Error.unauthorized) default: #if TESTABLE_BUILD diff --git a/SignalMessaging/groups/GroupsV2Protos.swift b/SignalMessaging/groups/GroupsV2Protos.swift index 6c6c1dcbb3..43448e314b 100644 --- a/SignalMessaging/groups/GroupsV2Protos.swift +++ b/SignalMessaging/groups/GroupsV2Protos.swift @@ -36,7 +36,7 @@ public class GroupsV2Protos { let builder = GroupsProtoMember.builder() builder.setRole(role) let presentationData = try self.presentationData(profileKeyCredential: profileKeyCredential, - groupV2Params: groupV2Params) + groupV2Params: groupV2Params) builder.setPresentation(presentationData) return try builder.build() @@ -73,6 +73,10 @@ public class GroupsV2Protos { groupV2Params: GroupV2Params, profileKeyCredentialMap: ProfileKeyCredentialMap, localUuid: UUID) throws -> GroupsProtoGroup { + + guard let groupModel = groupModel as? TSGroupModelV2 else { + throw OWSAssertionError("Invalid groupModel.") + } // Collect credential for self. guard let localProfileKeyCredential = profileKeyCredentialMap[localUuid] else { throw OWSAssertionError("Missing localProfileKeyCredential.") @@ -83,8 +87,17 @@ public class GroupsV2Protos { // GroupsV2 TODO: Constant-ize, revisit. groupBuilder.setVersion(0) groupBuilder.setPublicKey(groupV2Params.groupPublicParamsData) - groupBuilder.setTitle(try groupV2Params.encryptString(groupModel.groupName ?? "")) - // GroupsV2 TODO: Avatar + // GroupsV2 TODO: Will production implementation of encryptString() pad? + groupBuilder.setTitle(try groupV2Params.encryptString(groupModel.groupName?.stripped ?? " ")) + + let hasAvatarUrl = groupModel.groupAvatarUrlPath != nil + let hasAvatarData = groupModel.groupAvatarData != nil + guard hasAvatarData == hasAvatarUrl else { + throw OWSAssertionError("hasAvatarData: (\(hasAvatarData)) != hasAvatarUrl: (\(hasAvatarUrl))") + } + if let avatarUrl = groupModel.groupAvatarUrlPath { + groupBuilder.setAvatar(avatarUrl) + } groupBuilder.setAccessControl(try buildAccessProto(groupAccess: groupModel.groupAccess)) @@ -192,23 +205,29 @@ public class GroupsV2Protos { // GroupsV2 TODO: How can we make this parsing less brittle? public class func parse(groupProto: GroupsProtoGroup, + downloadedAvatars: GroupV2DownloadedAvatars, groupV2Params: GroupV2Params) throws -> GroupV2Snapshot { + var title = "" + if let titleData = groupProto.title { + do { + title = try groupV2Params.decryptString(titleData) + } catch { + owsFailDebug("Could not decrypt title: \(error).") + } + } + + var avatarUrlPath: String? + var avatarData: Data? + if let avatar = groupProto.avatar, !avatar.isEmpty { + avatarUrlPath = avatar + avatarData = try downloadedAvatars.avatarData(for: avatar) + } + // This client can learn of profile keys from parsing group state protos. // After parsing, we should fill in profileKeys in the profile manager. var profileKeys = [UUID: Data]() - // GroupsV2 TODO: Is GroupsProtoAccessControl required? - guard let accessControl = groupProto.accessControl else { - throw OWSAssertionError("Missing accessControl.") - } - guard let accessControlForAttributes = accessControl.attributes else { - throw OWSAssertionError("Missing accessControl.members.") - } - guard let accessControlForMembers = accessControl.members else { - throw OWSAssertionError("Missing accessControl.members.") - } - var members = [GroupV2SnapshotImpl.Member]() for memberProto in groupProto.members { guard let userID = memberProto.userID else { @@ -268,17 +287,16 @@ public class GroupsV2Protos { pendingMembers.append(pendingMember) } - var title = "" - if let titleData = groupProto.title { - do { - title = try groupV2Params.decryptString(titleData) - } catch { - owsFailDebug("Could not decrypt title: \(error).") - } + // GroupsV2 TODO: Is GroupsProtoAccessControl required? + guard let accessControl = groupProto.accessControl else { + throw OWSAssertionError("Missing accessControl.") + } + guard let accessControlForAttributes = accessControl.attributes else { + throw OWSAssertionError("Missing accessControl.members.") + } + guard let accessControlForMembers = accessControl.members else { + throw OWSAssertionError("Missing accessControl.members.") } - - // GroupsV2 TODO: Avatar - // public var avatar: String? { var disappearingMessageToken = DisappearingMessageToken.disabledToken if let disappearingMessagesTimerEncrypted = groupProto.disappearingMessagesTimer { @@ -299,6 +317,8 @@ public class GroupsV2Protos { groupProto: groupProto, revision: revision, title: title, + avatarUrlPath: avatarUrlPath, + avatarData: avatarData, members: members, pendingMembers: pendingMembers, accessControlForAttributes: accessControlForAttributes, @@ -311,6 +331,7 @@ public class GroupsV2Protos { // We do not treat an empty response with no changes as an error. public class func parse(groupChangesProto: GroupsProtoGroupChanges, + downloadedAvatars: GroupV2DownloadedAvatars, groupV2Params: GroupV2Params) throws -> [GroupV2Change] { var result = [GroupV2Change]() for changeStateProto in groupChangesProto.groupChanges { @@ -318,6 +339,7 @@ public class GroupsV2Protos { throw OWSAssertionError("Missing groupState proto.") } let snapshot = try parse(groupProto: snapshotProto, + downloadedAvatars: downloadedAvatars, groupV2Params: groupV2Params) guard let changeProto = changeStateProto.groupChange else { throw OWSAssertionError("Missing groupChange proto.") @@ -327,4 +349,62 @@ public class GroupsV2Protos { } return result } + + // MARK: - + + public class func collectAvatarUrlPaths(groupProto: GroupsProtoGroup? = nil, + groupChangesProto: GroupsProtoGroupChanges? = nil, + changeActionsProto: GroupsProtoGroupChangeActions? = nil, + groupV2Params: GroupV2Params) -> Promise<[String]> { + return DispatchQueue.global().async(.promise) { () throws -> [String] in + var avatarUrlPaths = [String]() + if let groupProto = groupProto { + avatarUrlPaths += self.collectAvatarUrlPaths(groupProto: groupProto) + } + if let groupChangesProto = groupChangesProto { + avatarUrlPaths += try self.collectAvatarUrlPaths(groupChangesProto: groupChangesProto, + groupV2Params: groupV2Params) + } + if let changeActionsProto = changeActionsProto { + avatarUrlPaths += self.collectAvatarUrlPaths(changeActionsProto: changeActionsProto) + } + // Discard empty avatar urls. + return avatarUrlPaths.filter { $0.count > 0 } + } + } + + private class func collectAvatarUrlPaths(groupChangesProto: GroupsProtoGroupChanges, + groupV2Params: GroupV2Params) throws -> [String] { + var avatarUrlPaths = [String]() + for changeStateProto in groupChangesProto.groupChanges { + guard let groupState = changeStateProto.groupState else { + throw OWSAssertionError("Missing groupState proto.") + } + avatarUrlPaths += collectAvatarUrlPaths(groupProto: groupState) + + guard let changeProto = changeStateProto.groupChange else { + throw OWSAssertionError("Missing groupChange proto.") + } + let changeActionsProto = try parseAndVerifyChangeActionsProto(changeProto, ignoreSignature: true) + avatarUrlPaths += self.collectAvatarUrlPaths(changeActionsProto: changeActionsProto) + } + return avatarUrlPaths + } + + private class func collectAvatarUrlPaths(changeActionsProto: GroupsProtoGroupChangeActions) -> [String] { + guard let modifyAvatarAction = changeActionsProto.modifyAvatar else { + return [] + } + guard let avatarUrlPath = modifyAvatarAction.avatar else { + return [] + } + return [avatarUrlPath] + } + + private class func collectAvatarUrlPaths(groupProto: GroupsProtoGroup) -> [String] { + guard let avatarUrlPath = groupProto.avatar else { + return [] + } + return [avatarUrlPath] + } } diff --git a/SignalMessaging/groups/GroupsV2Test.swift b/SignalMessaging/groups/GroupsV2Test.swift index 09cb58a911..536889b4c9 100644 --- a/SignalMessaging/groups/GroupsV2Test.swift +++ b/SignalMessaging/groups/GroupsV2Test.swift @@ -40,6 +40,11 @@ public class GroupsV2Test: NSObject { let members = [SignalServiceAddress]() let title0 = "hello" let title1 = "goodbye" + let avatar1Image = UIImage(color: .red, size: CGSize(width: 1, height: 1)) + guard let avatar1Data = avatar1Image.pngData() else { + owsFailDebug("Invalid avatar1Data.") + return + } guard let localUuid = tsAccountManager.localUuid else { owsFailDebug("Missing localUuid.") return @@ -64,19 +69,17 @@ public class GroupsV2Test: NSObject { GroupManager.localCreateNewGroup(members: members, name: title0, shouldSendMessage: true) - }.then(on: .global()) { (groupThread: TSGroupThread) -> Promise<(Data, GroupV2Snapshot)> in - let groupModel = groupThread.groupModel - guard groupModel.groupsVersion == .V2 else { - throw OWSAssertionError("Not a V2 group.") - } + }.then(on: .global()) { (groupThread: TSGroupThread) -> Promise in + let groupModel = groupThread.groupModel + guard groupModel.groupsVersion == .V2 else { + throw OWSAssertionError("Not a V2 group.") + } return self.groupsV2.fetchCurrentGroupV2Snapshot(groupModel: groupModel) - .map(on: .global()) { (groupV2Snapshot: GroupV2Snapshot) -> (Data, GroupV2Snapshot) in - return (groupThread.groupModel.groupId, groupV2Snapshot) - } - }.map(on: .global()) { (groupId: Data, groupV2Snapshot: GroupV2Snapshot) throws -> Data in + }.map(on: .global()) { (groupV2Snapshot: GroupV2Snapshot) throws -> Data in let groupModel = try self.databaseStorage.read { transaction in return try TSGroupModelBuilder(groupV2Snapshot: groupV2Snapshot).build(transaction: transaction) } + let groupId = groupModel.groupId guard groupModel.groupV2Revision == 0 else { throw OWSAssertionError("Unexpected groupV2Revision: \(groupModel.groupV2Revision).") } @@ -87,6 +90,15 @@ public class GroupsV2Test: NSObject { guard groupV2Snapshot.title == title0 else { throw OWSAssertionError("Unexpected group title: \(groupV2Snapshot.title).") } + guard groupV2Snapshot.avatarData == nil else { + throw OWSAssertionError("Unexpected group avatarData: \(groupV2Snapshot.avatarData?.hexadecimalString).") + } + guard groupModel.groupName == title0 else { + throw OWSAssertionError("Unexpected group title: \(groupModel.groupName).") + } + guard groupModel.groupAvatarData == nil else { + throw OWSAssertionError("Unexpected group avatarData: \(groupModel.groupAvatarData?.hexadecimalString).") + } let groupMembership = groupModel.groupMembership @@ -106,7 +118,7 @@ public class GroupsV2Test: NSObject { throw OWSAssertionError("Unexpected accessControlForAttributes: \(groupV2Snapshot.accessControlForAttributes).") } return groupId - }.then(on: .global()) { (groupId: Data) throws -> Promise<(Data, GroupV2Snapshot)> in + }.then(on: .global()) { (groupId: Data) throws -> Promise in let (groupThread, dmConfiguration) = try self.fetchGroupThread(groupId: groupId) let groupModel = groupThread.groupModel guard groupModel.groupMembership.administrators == localAddressSet else { @@ -118,6 +130,12 @@ public class GroupsV2Test: NSObject { guard groupModel.groupV2Revision == 0 else { throw OWSAssertionError("Unexpected groupV2Revision: \(groupModel.groupV2Revision).") } + guard groupModel.groupName == title0 else { + throw OWSAssertionError("Unexpected group title: \(groupModel.groupName).") + } + guard groupModel.groupAvatarData == nil else { + throw OWSAssertionError("Unexpected group avatarData: \(groupModel.groupAvatarData?.hexadecimalString).") + } var groupMembershipBuilder = groupModel.groupMembership.asBuilder for address in otherAddresses { @@ -131,22 +149,19 @@ public class GroupsV2Test: NSObject { return GroupManager.localUpdateExistingGroup(groupId: groupId, name: title1, - avatarData: nil, + avatarData: avatar1Data, groupMembership: groupMembership, groupAccess: groupAccess, groupsVersion: groupModel.groupsVersion, dmConfiguration: dmConfiguration, groupUpdateSourceAddress: localAddress) - .then(on: .global()) { (groupThread) -> Promise in - // GroupsV2 TODO: This should reflect the new group. - return groupsV2.fetchCurrentGroupV2Snapshot(groupModel: groupThread.groupModel) - }.map(on: .global()) { (groupV2Snapshot: GroupV2Snapshot) -> (Data, GroupV2Snapshot) in - return (groupId, groupV2Snapshot) - } - }.map(on: .global()) { (groupId: Data, groupV2Snapshot: GroupV2Snapshot) throws -> Data in + }.then(on: .global()) { (groupThread) -> Promise in + return self.groupsV2.fetchCurrentGroupV2Snapshot(groupModel: groupThread.groupModel) + }.map(on: .global()) { (groupV2Snapshot: GroupV2Snapshot) throws -> Data in let groupModel = try self.databaseStorage.read { transaction in return try TSGroupModelBuilder(groupV2Snapshot: groupV2Snapshot).build(transaction: transaction) } + let groupId = groupModel.groupId guard groupModel.groupV2Revision == 1 else { throw OWSAssertionError("Unexpected groupV2Revision: \(groupModel.groupV2Revision).") } @@ -157,6 +172,15 @@ public class GroupsV2Test: NSObject { guard groupV2Snapshot.title == title1 else { throw OWSAssertionError("Unexpected group title: \(groupV2Snapshot.title).") } + guard groupV2Snapshot.avatarData == avatar1Data else { + throw OWSAssertionError("Unexpected group avatarData: \(groupV2Snapshot.avatarData?.hexadecimalString).") + } + guard groupModel.groupName == title1 else { + throw OWSAssertionError("Unexpected group title: \(groupModel.groupName).") + } + guard groupModel.groupAvatarData == avatar1Data else { + throw OWSAssertionError("Unexpected group avatarData: \(groupModel.groupAvatarData?.hexadecimalString).") + } let groupMembership = groupModel.groupMembership @@ -177,7 +201,7 @@ public class GroupsV2Test: NSObject { throw OWSAssertionError("Unexpected accessControlForAttributes: \(groupV2Snapshot.accessControlForAttributes).") } return groupId - }.then(on: .global()) { (groupId: Data) throws -> Promise<(Data, GroupV2Snapshot)> in + }.then(on: .global()) { (groupId: Data) throws -> Promise in let (groupThread, dmConfiguration) = try self.fetchGroupThread(groupId: groupId) let groupModel = groupThread.groupModel guard groupModel.groupMembership.administrators == localAddressSet else { @@ -189,6 +213,12 @@ public class GroupsV2Test: NSObject { guard groupModel.groupV2Revision == 1 else { throw OWSAssertionError("Unexpected groupV2Revision: \(groupModel.groupV2Revision).") } + guard groupModel.groupName == title1 else { + throw OWSAssertionError("Unexpected group title: \(groupModel.groupName).") + } + guard groupModel.groupAvatarData == avatar1Data else { + throw OWSAssertionError("Unexpected group avatarData: \(groupModel.groupAvatarData?.hexadecimalString).") + } var groupMembershipBuilder = groupModel.groupMembership.asBuilder for address in otherAddresses { @@ -200,23 +230,20 @@ public class GroupsV2Test: NSObject { // GroupsV2 TODO: Add and remove members, change avatar, etc. return GroupManager.localUpdateExistingGroup(groupId: groupId, - name: title1, - avatarData: nil, - groupMembership: groupMembership, - groupAccess: groupAccess, - groupsVersion: groupModel.groupsVersion, - dmConfiguration: dmConfiguration, - groupUpdateSourceAddress: localAddress) - .then(on: .global()) { (_) -> Promise in - // GroupsV2 TODO: This should reflect the new group. - return groupsV2.fetchCurrentGroupV2Snapshot(groupModel: groupModel) - }.map(on: .global()) { (groupV2Snapshot: GroupV2Snapshot) -> (Data, GroupV2Snapshot) in - return (groupId, groupV2Snapshot) - } - }.map(on: .global()) { (groupId: Data, groupV2Snapshot: GroupV2Snapshot) throws -> Data in + name: title1, + avatarData: nil, + groupMembership: groupMembership, + groupAccess: groupAccess, + groupsVersion: groupModel.groupsVersion, + dmConfiguration: dmConfiguration, + groupUpdateSourceAddress: localAddress) + }.then(on: .global()) { (groupThread) -> Promise in + return self.groupsV2.fetchCurrentGroupV2Snapshot(groupModel: groupThread.groupModel) + }.map(on: .global()) { (groupV2Snapshot: GroupV2Snapshot) throws -> Data in let groupModel = try self.databaseStorage.read { transaction in return try TSGroupModelBuilder(groupV2Snapshot: groupV2Snapshot).build(transaction: transaction) } + let groupId: Data = groupModel.groupId guard groupModel.groupV2Revision == 2 else { throw OWSAssertionError("Unexpected groupV2Revision: \(groupModel.groupV2Revision).") } @@ -227,6 +254,15 @@ public class GroupsV2Test: NSObject { guard groupV2Snapshot.title == title1 else { throw OWSAssertionError("Unexpected group title: \(groupV2Snapshot.title).") } + guard groupV2Snapshot.avatarData == nil else { + throw OWSAssertionError("Unexpected group avatarData: \(groupV2Snapshot.avatarData?.hexadecimalString).") + } + guard groupModel.groupName == title1 else { + throw OWSAssertionError("Unexpected group title: \(groupModel.groupName).") + } + guard groupModel.groupAvatarData == nil else { + throw OWSAssertionError("Unexpected group avatarData: \(groupModel.groupAvatarData?.hexadecimalString).") + } let groupMembership = groupModel.groupMembership @@ -259,6 +295,12 @@ public class GroupsV2Test: NSObject { guard groupModel.groupV2Revision == 2 else { throw OWSAssertionError("Unexpected groupV2Revision: \(groupModel.groupV2Revision).") } + guard groupModel.groupName == title1 else { + throw OWSAssertionError("Unexpected group title: \(groupModel.groupName).") + } + guard groupModel.groupAvatarData == nil else { + throw OWSAssertionError("Unexpected group avatarData: \(groupModel.groupAvatarData?.hexadecimalString).") + } return groupId }.done { (_: Data) -> Void in Logger.info("---- Success.") diff --git a/SignalMessaging/groups/StorageService+GroupsV2.swift b/SignalMessaging/groups/StorageService+GroupsV2.swift index 10c45656f0..a5ee964ae4 100644 --- a/SignalMessaging/groups/StorageService+GroupsV2.swift +++ b/SignalMessaging/groups/StorageService+GroupsV2.swift @@ -64,6 +64,19 @@ public extension StorageService { authCredential: authCredential) } + static func buildGroupAvatarUploadFormRequest(groupV2Params: GroupV2Params, + sessionManager: AFHTTPSessionManager, + authCredential: AuthCredential) throws -> NSURLRequest { + + let urlPath = "/v1/groups/avatar/form" + return try buildGroupV2Request(protoData: nil, + urlPath: urlPath, + httpMethod: "GET", + groupV2Params: groupV2Params, + sessionManager: sessionManager, + authCredential: authCredential) + } + private static func buildGroupV2Request(protoData: Data?, urlPath: String, httpMethod: String, diff --git a/SignalMessaging/profiles/VersionedProfiles.swift b/SignalMessaging/profiles/VersionedProfiles.swift index 32433666ad..4ad23a3642 100644 --- a/SignalMessaging/profiles/VersionedProfiles.swift +++ b/SignalMessaging/profiles/VersionedProfiles.swift @@ -41,10 +41,6 @@ public class VersionedProfiles: NSObject { return SSKEnvironment.shared.networkManager } - private class var uploadHTTPManager: AFHTTPSessionManager { - return OWSSignalService.sharedInstance().cdnSessionManager - } - private class var databaseStorage: SDSDatabaseStorage { return SDSDatabaseStorage.shared } @@ -134,48 +130,19 @@ public class VersionedProfiles: NSObject { private class func parseFormAndUpload(formResponseObject: Any?, profileAvatarData: Data) -> Promise { - let urlPath: String = "" - let (promise, resolver) = Promise.pending() - DispatchQueue.global().async { + return firstly { () throws -> Promise in guard let response = formResponseObject as? [AnyHashable: Any] else { - resolver.reject(OWSAssertionError("Unexpected response.")) - return + throw OWSAssertionError("Unexpected response.") } - guard let form = OWSUploadForm.parse(response) else { - resolver.reject(OWSAssertionError("Could not parse response.")) - return + guard let form = OWSUploadForm.parseDictionary(response) else { + throw OWSAssertionError("Could not parse response.") } - - // TODO: We will probably (continue to) use this within profile manager. - let avatarUrlPath = form.formKey - - self.uploadHTTPManager.post(urlPath, - parameters: nil, - constructingBodyWith: { (formData: AFMultipartFormData) -> Void in - - // We have to build up the form manually vs. simply passing in a paramaters dict - // because AWS is sensitive to the order of the form params (at least the "key" - // field must occur early on). - // - // For consistency, all fields are ordered here in a known working order. - form.append(toForm: formData) - - AppendMultipartFormPath(formData, "Content-Type", OWSMimeTypeApplicationOctetStream) - - formData.appendPart(withForm: profileAvatarData, name: "file") - }, - progress: { progress in - Logger.verbose("progress: \(progress.fractionCompleted)") - }, - success: { (_, _) in - Logger.verbose("Success.") - resolver.fulfill(VersionedProfileUpdate(avatarUrlPath: avatarUrlPath)) - }, failure: { (_, error) in - owsFailDebug("Error: \(error)") - resolver.reject(error) - }) + return Promise.value(form) + }.then(on: DispatchQueue.global()) { (uploadForm: OWSUploadForm) -> Promise in + OWSUploadV2.upload(data: profileAvatarData, uploadForm: uploadForm, uploadUrlPath: "") + }.map(on: DispatchQueue.global()) { (avatarUrlPath: String) -> VersionedProfileUpdate in + return VersionedProfileUpdate(avatarUrlPath: avatarUrlPath) } - return promise } // MARK: - Get diff --git a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.h b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.h index f285199328..c285455498 100644 --- a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.h +++ b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.h @@ -58,13 +58,6 @@ NS_SWIFT_NAME(init(grdbId:uniqueId:conversationColorName:creationDate:isArchived + (NSArray *)groupThreadsWithAddress:(SignalServiceAddress *)address transaction:(SDSAnyReadTransaction *)transaction; -#pragma mark - Avatar - -// GroupsV2 TODO: Remove. -- (void)updateAvatarWithAttachmentStream:(TSAttachmentStream *)attachmentStream; -- (void)updateAvatarWithAttachmentStream:(TSAttachmentStream *)attachmentStream - transaction:(SDSAnyWriteTransaction *)transaction; - #pragma mark - Update With... // This method should only be called by GroupManager. diff --git a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m index 3772f20908..272202571e 100644 --- a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m +++ b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m @@ -169,46 +169,6 @@ NSString *const TSGroupThread_NotificationKey_UniqueId = @"TSGroupThread_Notific return NSLocalizedString(@"NEW_GROUP_DEFAULT_TITLE", @""); } -#pragma mark - Avatar - -- (void)updateAvatarWithAttachmentStream:(TSAttachmentStream *)attachmentStream -{ - [self.databaseStorage writeWithBlock:^(SDSAnyWriteTransaction *transaction) { - [self updateAvatarWithAttachmentStream:attachmentStream transaction:transaction]; - }]; -} - -- (void)updateAvatarWithAttachmentStream:(TSAttachmentStream *)attachmentStream - transaction:(SDSAnyWriteTransaction *)transaction -{ - OWSAssertDebug(attachmentStream); - OWSAssertDebug(transaction); - - [self anyUpdateGroupThreadWithTransaction:transaction - block:^(TSGroupThread *thread) { - NSData *_Nullable attachmentData = - [NSData dataWithContentsOfFile:attachmentStream.originalFilePath]; - if (attachmentData.length < 1) { - return; - } - if (thread.groupModel.groupAvatarData.length > 0 && - [thread.groupModel.groupAvatarData isEqualToData:attachmentData]) { - // Avatar did not change. - return; - } - UIImage *_Nullable avatarImage = [attachmentStream thumbnailImageSmallSync]; - [thread.groupModel setGroupAvatarDataWithImage:avatarImage]; - }]; - - [transaction addAsyncCompletion:^{ - [self fireAvatarChangedNotification]; - }]; - - // Avatars are stored directly in the database, so there's no need - // to keep the attachment around after assigning the image. - [attachmentStream anyRemoveWithTransaction:transaction]; -} - - (void)updateWithGroupModel:(TSGroupModel *)groupModel transaction:(SDSAnyWriteTransaction *)transaction { OWSAssertDebug(groupModel); diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 6338bd379c..8efaf58d5a 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -1108,8 +1108,10 @@ NS_ASSUME_NONNULL_BEGIN } if (groupContext.avatar != nil) { OWSLogVerbose(@"Data message had group avatar attachment"); + TSGroupThread *newGroupThread = result.groupThread; [self handleReceivedGroupAvatarUpdateWithEnvelope:envelope dataMessage:dataMessage + groupThread:newGroupThread transaction:transaction]; } @@ -1146,6 +1148,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)handleReceivedGroupAvatarUpdateWithEnvelope:(SSKProtoEnvelope *)envelope dataMessage:(SSKProtoDataMessage *)dataMessage + groupThread:(TSGroupThread *)groupThread transaction:(SDSAnyWriteTransaction *)transaction { if (!envelope) { @@ -1160,15 +1163,21 @@ NS_ASSUME_NONNULL_BEGIN OWSFail(@"Missing transaction."); return; } - - // GroupsV2 TODO: Handle groups v2. - TSGroupThread *_Nullable groupThread = [TSGroupThread fetchWithGroupId:dataMessage.group.id - transaction:transaction]; - if (!groupThread) { - OWSFailDebug(@"Missing group for group avatar update"); + if (groupThread.groupModel.groupsVersion != GroupsVersionV1) { + OWSFail(@"Invalid groupsVersion."); return; } + SignalServiceAddress *groupUpdateSourceAddress; + if (!envelope.sourceAddress.isValid) { + OWSFailDebug(@"Invalid envelope.sourceAddress"); + return; + } else { + groupUpdateSourceAddress = envelope.sourceAddress; + } + + NSData *groupId = groupThread.groupModel.groupId; + TSAttachmentPointer *_Nullable avatarPointer = [TSAttachmentPointer attachmentPointerFromProto:dataMessage.group.avatar albumMessage:nil]; @@ -1185,9 +1194,32 @@ NS_ASSUME_NONNULL_BEGIN success:^(NSArray *attachmentStreams) { OWSAssertDebug(attachmentStreams.count == 1); TSAttachmentStream *attachmentStream = attachmentStreams.firstObject; + NSData *_Nullable avatarData = attachmentStream.validStillImageData; + if (avatarData == nil) { + OWSFailDebug(@"Missing avatarData."); + return; + } [self.databaseStorage writeWithBlock:^(SDSAnyWriteTransaction *transaction) { - [groupThread updateAvatarWithAttachmentStream:attachmentStream transaction:transaction]; + TSGroupThread *_Nullable oldGroupThread = [TSGroupThread fetchWithGroupId:groupId + transaction:transaction]; + if (oldGroupThread == nil) { + OWSFailDebug(@"Missing oldGroupThread."); + return; + } + NSError *_Nullable error; + UpsertGroupResult *_Nullable result = + [GroupManager remoteUpdateToExistingGroupV1WithGroupId:groupId + name:oldGroupThread.groupModel.groupName + avatarData:avatarData + groupMembership:oldGroupThread.groupModel.groupMembership + groupUpdateSourceAddress:groupUpdateSourceAddress + transaction:transaction + error:&error]; + if (error != nil || result == nil) { + OWSFailDebug(@"Error: %@", error); + return; + } // Eagerly clean up the attachment. [attachmentStream anyRemoveWithTransaction:transaction]; diff --git a/SignalServiceKit/src/Messages/Stickers/CDNDownloadOperation.swift b/SignalServiceKit/src/Messages/Stickers/CDNDownloadOperation.swift index de5f241d25..759798f40f 100644 --- a/SignalServiceKit/src/Messages/Stickers/CDNDownloadOperation.swift +++ b/SignalServiceKit/src/Messages/Stickers/CDNDownloadOperation.swift @@ -1,11 +1,11 @@ // -// Copyright (c) 2019 Open Whisper Systems. All rights reserved. +// Copyright (c) 2020 Open Whisper Systems. All rights reserved. // import Foundation import PromiseKit -class CDNDownloadOperation: OWSOperation { +open class CDNDownloadOperation: OWSOperation { // MARK: - Dependencies @@ -37,7 +37,7 @@ class CDNDownloadOperation: OWSOperation { let kMaxStickerDownloadSize: UInt = 100 * 1000 - func tryToDownload(urlPath: String, maxDownloadSize: UInt) throws -> Promise { + public func tryToDownload(urlPath: String, maxDownloadSize: UInt?) throws -> Promise { guard !isCorrupt(urlPath: urlPath) else { Logger.warn("Skipping download of corrupt data.") throw StickerError.corruptData @@ -95,9 +95,11 @@ class CDNDownloadOperation: OWSOperation { // // If the current downloaded bytes or the expected total byes // exceed the max download size, abort the download. - guard progress.totalUnitCount <= maxDownloadSize, - progress.completedUnitCount <= maxDownloadSize else { - return abortDownload("Attachment download exceed expected content length: \(progress.totalUnitCount), \(progress.completedUnitCount).") + if let maxDownloadSize = maxDownloadSize { + guard progress.totalUnitCount <= maxDownloadSize, + progress.completedUnitCount <= maxDownloadSize else { + return abortDownload("Attachment download exceed expected content length: \(progress.totalUnitCount), \(progress.completedUnitCount).") + } } // We only need to check the content length header once. @@ -123,8 +125,10 @@ class CDNDownloadOperation: OWSOperation { guard let contentLength = Int64(contentLengthString) else { return abortDownload("Invalid content length.") } - guard contentLength < maxDownloadSize else { - return abortDownload("Content length exceeds max download size.") + if let maxDownloadSize = maxDownloadSize { + guard contentLength < maxDownloadSize else { + return abortDownload("Content length exceeds max download size.") + } } // This response has a valid content length that is less @@ -152,9 +156,11 @@ class CDNDownloadOperation: OWSOperation { owsFailDebug("Couldn't determine file size.") return resolver.reject(StickerError.assertionFailure) } - guard fileSize.uint64Value <= maxDownloadSize else { - owsFailDebug("Download length exceeds max size.") - return resolver.reject(StickerError.assertionFailure) + if let maxDownloadSize = maxDownloadSize { + guard fileSize.uint64Value <= maxDownloadSize else { + owsFailDebug("Download length exceeds max size.") + return resolver.reject(StickerError.assertionFailure) + } } do { diff --git a/SignalServiceKit/src/Network/API/NetworkManager.swift b/SignalServiceKit/src/Network/API/NetworkManager.swift index ef3f17e4b4..4180ab1857 100644 --- a/SignalServiceKit/src/Network/API/NetworkManager.swift +++ b/SignalServiceKit/src/Network/API/NetworkManager.swift @@ -86,3 +86,11 @@ public extension TSNetworkManager { } } } + +// MARK: - + +public extension Error { + var isNetworkConnectivityFailure: Bool { + return IsNetworkConnectivityFailure(self) + } +} diff --git a/SignalServiceKit/src/Network/API/OWSUploadV2.h b/SignalServiceKit/src/Network/API/OWSUploadV2.h index 792cea4b83..8198b55f11 100644 --- a/SignalServiceKit/src/Network/API/OWSUploadV2.h +++ b/SignalServiceKit/src/Network/API/OWSUploadV2.h @@ -15,20 +15,32 @@ void AppendMultipartFormPath(id formData, NSString *name, N @interface OWSUploadForm : NSObject -// These properties will bet set for all uploads. -@property (nonatomic) NSString *formAcl; -@property (nonatomic) NSString *formKey; -@property (nonatomic) NSString *formPolicy; -@property (nonatomic) NSString *formAlgorithm; -@property (nonatomic) NSString *formCredential; -@property (nonatomic) NSString *formDate; -@property (nonatomic) NSString *formSignature; +// These properties will be set for all uploads. +@property (nonatomic, readonly) NSString *acl; +@property (nonatomic, readonly) NSString *key; +@property (nonatomic, readonly) NSString *policy; +@property (nonatomic, readonly) NSString *algorithm; +@property (nonatomic, readonly) NSString *credential; +@property (nonatomic, readonly) NSString *date; +@property (nonatomic, readonly) NSString *signature; -// These properties will bet set for all attachment uploads. -@property (nonatomic, nullable) NSNumber *attachmentId; -@property (nonatomic, nullable) NSString *attachmentIdString; +// These properties will be set for all attachment uploads. +@property (nonatomic, readonly, nullable) NSNumber *attachmentId; +@property (nonatomic, readonly, nullable) NSString *attachmentIdString; -+ (nullable OWSUploadForm *)parse:(nullable NSDictionary *)formResponseObject; +- (instancetype)init NS_UNAVAILABLE; + +- (instancetype)initWithAcl:(NSString *)acl + key:(NSString *)key + policy:(NSString *)policy + algorithm:(NSString *)algorithm + credential:(NSString *)credential + date:(NSString *)date + signature:(NSString *)signature + attachmentId:(nullable NSNumber *)attachmentId + attachmentIdString:(nullable NSString *)attachmentIdString NS_DESIGNATED_INITIALIZER; + ++ (nullable OWSUploadForm *)parseDictionary:(nullable NSDictionary *)formResponseObject; - (void)appendToForm:(id)formData; diff --git a/SignalServiceKit/src/Network/API/OWSUploadV2.m b/SignalServiceKit/src/Network/API/OWSUploadV2.m index 8777343038..bb542a0660 100644 --- a/SignalServiceKit/src/Network/API/OWSUploadV2.m +++ b/SignalServiceKit/src/Network/API/OWSUploadV2.m @@ -30,7 +30,33 @@ void AppendMultipartFormPath(id formData, NSString *name, N // See: https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-UsingHTTPPOST.html @implementation OWSUploadForm -+ (nullable OWSUploadForm *)parse:(nullable NSDictionary *)formResponseObject +- (instancetype)initWithAcl:(NSString *)acl + key:(NSString *)key + policy:(NSString *)policy + algorithm:(NSString *)algorithm + credential:(NSString *)credential + date:(NSString *)date + signature:(NSString *)signature + attachmentId:(nullable NSNumber *)attachmentId + attachmentIdString:(nullable NSString *)attachmentIdString +{ + self = [super init]; + + if (self) { + _acl = acl; + _key = key; + _policy = policy; + _algorithm = algorithm; + _credential = credential; + _date = date; + _signature = signature; + _attachmentId = attachmentId; + _attachmentIdString = attachmentIdString; + } + return self; +} + ++ (nullable OWSUploadForm *)parseDictionary:(nullable NSDictionary *)formResponseObject { if (![formResponseObject isKindOfClass:[NSDictionary class]]) { OWSFailDebug(@"Invalid upload form."); @@ -89,22 +115,15 @@ void AppendMultipartFormPath(id formData, NSString *name, N return nil; } - OWSUploadForm *form = [OWSUploadForm new]; - - // Required properties. - form.formAcl = formAcl; - form.formKey = formKey; - form.formPolicy = formPolicy; - form.formAlgorithm = formAlgorithm; - form.formCredential = formCredential; - form.formDate = formDate; - form.formSignature = formSignature; - - // Optional properties. - form.attachmentId = attachmentId; - form.attachmentIdString = attachmentIdString; - - return form; + return [[OWSUploadForm alloc] initWithAcl:formAcl + key:formKey + policy:formPolicy + algorithm:formAlgorithm + credential:formCredential + date:formDate + signature:formSignature + attachmentId:attachmentId + attachmentIdString:attachmentIdString]; } - (void)appendToForm:(id)formData @@ -114,13 +133,13 @@ void AppendMultipartFormPath(id formData, NSString *name, N // field must occur early on). // // For consistency, all fields are ordered here in a known working order. - AppendMultipartFormPath(formData, @"key", self.formKey); - AppendMultipartFormPath(formData, @"acl", self.formAcl); - AppendMultipartFormPath(formData, @"x-amz-algorithm", self.formAlgorithm); - AppendMultipartFormPath(formData, @"x-amz-credential", self.formCredential); - AppendMultipartFormPath(formData, @"x-amz-date", self.formDate); - AppendMultipartFormPath(formData, @"policy", self.formPolicy); - AppendMultipartFormPath(formData, @"x-amz-signature", self.formSignature); + AppendMultipartFormPath(formData, @"key", self.key); + AppendMultipartFormPath(formData, @"acl", self.acl); + AppendMultipartFormPath(formData, @"x-amz-algorithm", self.algorithm); + AppendMultipartFormPath(formData, @"x-amz-credential", self.credential); + AppendMultipartFormPath(formData, @"x-amz-date", self.date); + AppendMultipartFormPath(formData, @"policy", self.policy); + AppendMultipartFormPath(formData, @"x-amz-signature", self.signature); } @end @@ -175,7 +194,7 @@ void AppendMultipartFormPath(id formData, NSString *name, N } // TODO: Should we use a non-empty urlPath? - [[strongSelf parseFormAndUpload:formResponseObject urlPath:@"" progressBlock:progressBlock] + [[strongSelf parseFormAndUpload:formResponseObject progressBlock:progressBlock] .thenInBackground(^{ return resolve(@(1)); }) @@ -194,61 +213,18 @@ void AppendMultipartFormPath(id formData, NSString *name, N } - (AnyPromise *)parseFormAndUpload:(nullable id)formResponseObject - urlPath:(NSString *)urlPath progressBlock:(UploadProgressBlock)progressBlock { - OWSUploadForm *_Nullable form = [OWSUploadForm parse:formResponseObject]; + OWSUploadForm *_Nullable form = [OWSUploadForm parseDictionary:formResponseObject]; if (!form) { return [AnyPromise promiseWithValue:OWSErrorWithCodeDescription(OWSErrorCodeUploadFailed, @"Invalid upload form.")]; } - self.urlPath = form.formKey; + self.urlPath = form.key; - __weak OWSAvatarUploadV2 *weakSelf = self; - AnyPromise *promise = [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) { - [self.uploadHTTPManager POST:urlPath - parameters:nil - constructingBodyWithBlock:^(id formData) { - OWSAvatarUploadV2 *_Nullable strongSelf = weakSelf; - if (!strongSelf) { - return resolve(OWSErrorWithCodeDescription(OWSErrorCodeUploadFailed, @"Upload deallocated")); - } - - // We have to build up the form manually vs. simply passing in a paramaters dict - // because AWS is sensitive to the order of the form params (at least the "key" - // field must occur early on). - // - // For consistency, all fields are ordered here in a known working order. - [form appendToForm:formData]; - AppendMultipartFormPath(formData, @"Content-Type", OWSMimeTypeApplicationOctetStream); - - NSData *_Nullable uploadData = strongSelf.avatarData; - if (uploadData.length < 1) { - OWSCFailDebug(@"Could not load upload data."); - return resolve( - OWSErrorWithCodeDescription(OWSErrorCodeUploadFailed, @"Could not load upload data.")); - } - OWSCAssertDebug(uploadData.length > 0); - [formData appendPartWithFormData:uploadData name:@"file"]; - - OWSLogVerbose(@"constructed body"); - } - progress:^(NSProgress *progress) { - OWSLogVerbose(@"Upload progress: %.2f%%", progress.fractionCompleted * 100); - - progressBlock(progress); - } - success:^(NSURLSessionDataTask *uploadTask, id _Nullable responseObject) { - OWSLogInfo(@"Upload succeeded with key: %@", form.formKey); - return resolve(@(1)); - } - failure:^(NSURLSessionDataTask *_Nullable uploadTask, NSError *error) { - OWSLogError(@"Upload failed with error: %@", error); - resolve(error); - }]; - }]; - return promise; + NSString *uploadUrlPath = @""; + return [OWSUploadV2 uploadObjcWithData:self.avatarData uploadForm:form uploadUrlPath:uploadUrlPath]; } @end @@ -343,7 +319,7 @@ void AppendMultipartFormPath(id formData, NSString *name, N return resolve(OWSErrorWithCodeDescription(OWSErrorCodeUploadFailed, @"Upload deallocated")); } - [[strongSelf parseFormAndUpload:formResponseObject urlPath:@"attachments/" progressBlock:progressBlock] + [[strongSelf parseFormAndUpload:formResponseObject progressBlock:progressBlock] .thenInBackground(^{ resolve(@(1)); }) @@ -381,10 +357,9 @@ void AppendMultipartFormPath(id formData, NSString *name, N #pragma mark - - (AnyPromise *)parseFormAndUpload:(nullable id)formResponseObject - urlPath:(NSString *)urlPath progressBlock:(UploadProgressBlock)progressBlock { - OWSUploadForm *_Nullable form = [OWSUploadForm parse:formResponseObject]; + OWSUploadForm *_Nullable form = [OWSUploadForm parseDictionary:formResponseObject]; if (!form) { return [AnyPromise promiseWithValue:OWSErrorWithCodeDescription(OWSErrorCodeUploadFailed, @"Invalid upload form.")]; @@ -397,55 +372,8 @@ void AppendMultipartFormPath(id formData, NSString *name, N self.serverId = serverId; - __weak OWSAttachmentUploadV2 *weakSelf = self; - AnyPromise *promise = [AnyPromise promiseWithResolverBlock:^(PMKResolver resolve) { - [self.uploadHTTPManager POST:urlPath - parameters:nil - constructingBodyWithBlock:^(id formData) { - OWSAttachmentUploadV2 *_Nullable strongSelf = weakSelf; - if (!strongSelf) { - return resolve(OWSErrorWithCodeDescription(OWSErrorCodeUploadFailed, @"Upload deallocated")); - } - - // We have to build up the form manually vs. simply passing in a paramaters dict - // because AWS is sensitive to the order of the form params (at least the "key" - // field must occur early on). - // - // For consistency, all fields are ordered here in a known working order. - [form appendToForm:formData]; - AppendMultipartFormPath(formData, @"Content-Type", OWSMimeTypeApplicationOctetStream); - - NSData *_Nullable uploadData = [strongSelf attachmentData]; - if (uploadData.length < 1) { - OWSCFailDebug(@"Could not load upload data."); - return resolve( - OWSErrorWithCodeDescription(OWSErrorCodeUploadFailed, @"Could not load upload data.")); - } - OWSAssertDebug(uploadData.length > 0); - [formData appendPartWithFormData:uploadData name:@"file"]; - - OWSLogVerbose(@"constructed %@ body", [NSByteCountFormatter stringFromByteCount:uploadData.length countStyle:NSByteCountFormatterCountStyleFile]); - } - progress:^(NSProgress *progress) { - OWSLogVerbose(@"Upload progress: %.2f%%", progress.fractionCompleted * 100); - - progressBlock(progress); - } - success:^(NSURLSessionDataTask *uploadTask, id _Nullable responseObject) { - OWSAttachmentUploadV2 *_Nullable strongSelf = weakSelf; - if (!strongSelf) { - return resolve(OWSErrorWithCodeDescription(OWSErrorCodeUploadFailed, @"Upload deallocated")); - } - - OWSLogInfo(@"Upload succeeded with key: %@", form.formKey); - return resolve(@(1)); - } - failure:^(NSURLSessionDataTask *_Nullable uploadTask, NSError *error) { - OWSLogError(@"Upload failed with error: %@", error); - resolve(error); - }]; - }]; - return promise; + NSString *uploadUrlPath = @"attachments/"; + return [OWSUploadV2 uploadObjcWithData:self.attachmentData uploadForm:form uploadUrlPath:uploadUrlPath]; } @end diff --git a/SignalServiceKit/src/Network/API/OWSUploadV2.swift b/SignalServiceKit/src/Network/API/OWSUploadV2.swift new file mode 100644 index 0000000000..5fca48ab04 --- /dev/null +++ b/SignalServiceKit/src/Network/API/OWSUploadV2.swift @@ -0,0 +1,107 @@ +// +// Copyright (c) 2020 Open Whisper Systems. All rights reserved. +// + +import Foundation +import PromiseKit + +@objc +public class OWSUploadV2: NSObject { + @objc + public class func uploadObjc(data: Data, uploadForm: OWSUploadForm, uploadUrlPath: String) -> AnyPromise { + return AnyPromise(upload(data: data, uploadForm: uploadForm, uploadUrlPath: uploadUrlPath)) + } +} + +// MARK: - + +public extension OWSUploadV2 { + + // MARK: - Dependencies + + private class var uploadSessionManager: AFHTTPSessionManager { + return OWSSignalService.sharedInstance().cdnSessionManager + } + + // MARK: - + + class func upload(data: Data, uploadForm: OWSUploadForm, uploadUrlPath: String) -> Promise { + let (promise, resolver) = Promise.pending() + DispatchQueue.global().async { + self.uploadSessionManager.post(uploadUrlPath, + parameters: nil, + constructingBodyWith: { (formData: AFMultipartFormData) -> Void in + + // We have to build up the form manually vs. simply passing in a parameters dict + // because AWS is sensitive to the order of the form params (at least the "key" + // field must occur early on). + // + // For consistency, all fields are ordered here in a known working order. + uploadForm.append(toForm: formData) + + AppendMultipartFormPath(formData, "Content-Type", OWSMimeTypeApplicationOctetStream) + + formData.appendPart(withForm: data, name: "file") + }, + progress: { progress in + Logger.verbose("progress: \(progress.fractionCompleted)") + }, + success: { (_, _) in + Logger.verbose("Success.") + let uploadedUrlPath = uploadForm.key + resolver.fulfill(uploadedUrlPath) + }, failure: { (task, error) in + if let task = task { + #if TESTABLE_BUILD + TSNetworkManager.logCurl(for: task) + #endif + } else { + owsFailDebug("Missing task.") + } + + owsFailDebug("Error: \(error)") + resolver.reject(error) + }) + } + return promise + } +} + +// MARK: - + +public extension OWSUploadForm { + class func parse(proto attributesProto: GroupsProtoAvatarUploadAttributes) throws -> OWSUploadForm { + + guard let acl = attributesProto.acl else { + throw OWSAssertionError("Missing acl.") + } + guard let key = attributesProto.key else { + throw OWSAssertionError("Missing key.") + } + guard let policy = attributesProto.policy else { + throw OWSAssertionError("Missing policy.") + } + guard let algorithm = attributesProto.algorithm else { + throw OWSAssertionError("Missing algorithm.") + } + guard let credential = attributesProto.credential else { + throw OWSAssertionError("Missing credential.") + } + guard let date = attributesProto.date else { + throw OWSAssertionError("Missing acl.") + } + guard let signature = attributesProto.signature else { + throw OWSAssertionError("Missing signature.") + } + + return OWSUploadForm(acl: acl, + key: key, + policy: policy, + algorithm: algorithm, + credential: credential, + date: date, + signature: signature, + attachmentId: nil, + attachmentIdString: nil) + } +} diff --git a/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift b/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift index a5185ee952..a92c0646ab 100644 --- a/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift +++ b/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift @@ -30,12 +30,12 @@ class IncomingGroupsV2MessageQueue: NSObject { return SDSDatabaseStorage.shared } - private var groupsV2: GroupsV2 { - return SSKEnvironment.shared.groupsV2 + private var groupsV2: GroupsV2Swift { + return SSKEnvironment.shared.groupsV2 as! GroupsV2Swift } - private var groupV2Updates: GroupV2Updates { - return SSKEnvironment.shared.groupV2Updates + private var groupV2Updates: GroupV2UpdatesSwift { + return SSKEnvironment.shared.groupV2Updates as! GroupV2UpdatesSwift } private var blockingManager: OWSBlockingManager { @@ -463,13 +463,14 @@ class IncomingGroupsV2MessageQueue: NSObject { } private func updateGroupPromise(jobInfo: IncomingGroupsV2MessageJobInfo) -> Promise { - // First, we try to update the group locally using changes embedded in // the group context (if any). - return databaseStorage.write(.promise) { (transaction) -> UpdateOutcome in - return self.tryToUpdateUsingEmbeddedGroupUpdate(jobInfo: jobInfo, - transaction: transaction) - }.then(on: DispatchQueue.global()) { (embeddedUpdateOutcome) -> Promise in + firstly { + return self.tryToUpdateUsingEmbeddedGroupUpdate(jobInfo: jobInfo) + }.recover(on: .global()) { _ in + owsFailDebug("tryToUpdateUsingEmbeddedGroupUpdate should never fail.") + return Guarantee.value(UpdateOutcome.failureShouldFailoverToService) + }.then(on: DispatchQueue.global()) { (embeddedUpdateOutcome: UpdateOutcome) -> Promise in if embeddedUpdateOutcome == .failureShouldFailoverToService { return self.tryToUpdateUsingService(jobInfo: jobInfo) } else { @@ -494,83 +495,96 @@ class IncomingGroupsV2MessageQueue: NSObject { // * Return .failureShouldDiscard if this message should be discarded. // // This method should never return .failureShouldRetry. - private func tryToUpdateUsingEmbeddedGroupUpdate(jobInfo: IncomingGroupsV2MessageJobInfo, - transaction: SDSAnyWriteTransaction) -> UpdateOutcome { - guard let groupContextInfo = jobInfo.groupContextInfo, - let groupContext = jobInfo.groupContext else { - owsFailDebug("Missing jobInfo properties.") - return .failureShouldDiscard - } - let groupId = groupContextInfo.groupId - guard let groupThread = TSGroupThread.fetch(groupId: groupId, transaction: transaction) else { - // We might be learning of a group for the first time - // in which case we should fetch current group state from the - // service. - return .failureShouldFailoverToService - } - let oldGroupModel = groupThread.groupModel - guard oldGroupModel.groupsVersion == .V2 else { - owsFailDebug("Invalid groupsVersion.") - return .failureShouldDiscard - } - guard groupContext.hasRevision else { - owsFailDebug("Missing revision.") - return .failureShouldDiscard - } - let contextRevision = groupContext.revision - guard contextRevision > oldGroupModel.groupV2Revision else { - // Group is already updated. - // No need to apply embedded change from the group context; it is obsolete. - // This can happen due to races. - return .successShouldProcess - } - guard contextRevision == oldGroupModel.groupV2Revision + 1 else { - // We can only apply embedded changes if we're behind exactly - // one revision. - return .failureShouldFailoverToService - } - guard FeatureFlags.groupsV2processProtosInGroupUpdates else { - return .failureShouldFailoverToService - } - guard let changeActionsProtoData = groupContext.groupChange else { - // No embedded group change. - return .failureShouldFailoverToService - } - let changeActionsProto: GroupsProtoGroupChangeActions - do { - // We need to verify the signature because this proto came from - // another client, not the service. - changeActionsProto = try groupsV2.parseAndVerifyChangeActionsProto(changeActionsProtoData, - ignoreSignature: false) - } catch { - owsFailDebug("Error: \(error)") - return .failureShouldFailoverToService - } + private func tryToUpdateUsingEmbeddedGroupUpdate(jobInfo: IncomingGroupsV2MessageJobInfo) -> Promise { + let (promise, resolver) = Promise.pending() + DispatchQueue.global().async { + guard let groupContextInfo = jobInfo.groupContextInfo, + let groupContext = jobInfo.groupContext else { + owsFailDebug("Missing jobInfo properties.") + return resolver.fulfill(.failureShouldDiscard) + } + let groupId = groupContextInfo.groupId + guard GroupManager.isValidGroupId(groupId, groupsVersion: .V2) else { + owsFailDebug("Invalid groupId.") + return resolver.fulfill(.failureShouldDiscard) + } + let thread = self.databaseStorage.read { transaction in + return TSGroupThread.fetch(groupId: groupId, transaction: transaction) + } + guard let groupThread = thread else { + // We might be learning of a group for the first time + // in which case we should fetch current group state from the + // service. + return resolver.fulfill(.failureShouldFailoverToService) + } + let oldGroupModel = groupThread.groupModel + guard oldGroupModel.groupsVersion == .V2 else { + owsFailDebug("Invalid groupsVersion.") + return resolver.fulfill(.failureShouldDiscard) + } + guard groupContext.hasRevision else { + owsFailDebug("Missing revision.") + return resolver.fulfill(.failureShouldDiscard) + } + let contextRevision = groupContext.revision + guard contextRevision > oldGroupModel.groupV2Revision else { + // Group is already updated. + // No need to apply embedded change from the group context; it is obsolete. + // This can happen due to races. + return resolver.fulfill(.successShouldProcess) + } + guard contextRevision == oldGroupModel.groupV2Revision + 1 else { + // We can only apply embedded changes if we're behind exactly + // one revision. + return resolver.fulfill(.failureShouldFailoverToService) + } + guard FeatureFlags.groupsV2processProtosInGroupUpdates else { + return resolver.fulfill(.failureShouldFailoverToService) + } + guard let changeActionsProtoData = groupContext.groupChange else { + // No embedded group change. + return resolver.fulfill(.failureShouldFailoverToService) + } - let updatedGroupThread: TSGroupThread - do { - updatedGroupThread = try groupV2Updates.updateGroupWithChangeActions(groupId: groupId, - changeActionsProto: changeActionsProto, - transaction: transaction) - } catch { - owsFailDebug("Error: \(error)") - // GroupsV2 TODO: Make sure this is still correct behavior. - return .failureShouldFailoverToService - } - let updatedGroupModel = updatedGroupThread.groupModel + DispatchQueue.global().async(.promise) { + // We need to verify the signature because this proto came from + // another client, not the service. + return try self.groupsV2.parseAndVerifyChangeActionsProto(changeActionsProtoData, + ignoreSignature: false) + }.then(on: .global()) { (changeActionsProto: GroupsProtoGroupChangeActions) throws -> Promise in + guard let groupSecretParamsData = oldGroupModel.groupSecretParamsData else { + throw OWSAssertionError("Missing groupSecretParamsData.") + } + return try self.groupsV2.updateGroupWithChangeActions(groupId: oldGroupModel.groupId, + changeActionsProto: changeActionsProto, + groupSecretParamsData: groupSecretParamsData) + }.map(on: .global()) { (updatedGroupThread: TSGroupThread) throws -> Void in + let updatedGroupModel = updatedGroupThread.groupModel - guard updatedGroupModel.groupV2Revision >= contextRevision else { - owsFailDebug("Invalid revision.") - return .failureShouldFailoverToService + guard updatedGroupModel.groupV2Revision >= contextRevision else { + owsFailDebug("Invalid revision.") + return resolver.fulfill(.failureShouldFailoverToService) + } + guard updatedGroupModel.groupV2Revision == contextRevision else { + // We expect the embedded changes to update us to the target + // revision. If we update past that, assert but proceed in production. + owsFailDebug("Unexpected revision.") + return resolver.fulfill(.successShouldProcess) + } + Logger.info("Successfully applied embedded change proto from group context.") + return resolver.fulfill(.successShouldProcess) + }.catch(on: .global()) { error in + if error.isNetworkConnectivityFailure { + Logger.warn("Error: \(error)") + return resolver.fulfill(.failureShouldRetry) + } else { + owsFailDebug("Error: \(error)") + // GroupsV2 TODO: Make sure this is still correct behavior. + return resolver.fulfill(.failureShouldFailoverToService) + } + }.retainUntilComplete() } - guard updatedGroupModel.groupV2Revision == contextRevision else { - // We expect the embedded changes to update us to the target - // revision. If we update past that, assert but proceed in production. - owsFailDebug("Unexpected revision.") - return .successShouldProcess - } - Logger.info("Successfully applied embedded change proto from group context.") - return .successShouldProcess + return promise } private func tryToUpdateUsingService(jobInfo: IncomingGroupsV2MessageJobInfo) -> Promise { diff --git a/SignalServiceKit/src/SignalServiceKit.h b/SignalServiceKit/src/SignalServiceKit.h index bd7d3c1077..9b8c2f0603 100644 --- a/SignalServiceKit/src/SignalServiceKit.h +++ b/SignalServiceKit/src/SignalServiceKit.h @@ -38,6 +38,7 @@ #import #import #import +#import #import #import #import diff --git a/SignalServiceKit/src/groups/GroupAccess.swift b/SignalServiceKit/src/groups/GroupAccess.swift index 2a58e820ed..878be63b7f 100644 --- a/SignalServiceKit/src/groups/GroupAccess.swift +++ b/SignalServiceKit/src/groups/GroupAccess.swift @@ -74,11 +74,6 @@ public class GroupAccess: MTLModel { return allAccess } - @objc - public static var defaultForV2: GroupAccess { - return GroupAccess(members: .member, attributes: .member) - } - public class func groupV2Access(forProtoAccess value: GroupsProtoAccessControlAccessRequired) -> GroupV2Access { switch value { case .any: diff --git a/SignalServiceKit/src/groups/GroupManager.swift b/SignalServiceKit/src/groups/GroupManager.swift index 3b841b5335..612c9de969 100644 --- a/SignalServiceKit/src/groups/GroupManager.swift +++ b/SignalServiceKit/src/groups/GroupManager.swift @@ -168,6 +168,7 @@ public class GroupManager: NSObject { // // "New" groups are being created for the first time; they might need to be created on the service. + // NOTE: groupId param should only be set for tests. public static func localCreateNewGroup(members: [SignalServiceAddress], groupId: Data? = nil, name: String? = nil, @@ -187,6 +188,7 @@ public class GroupManager: NSObject { } } + // NOTE: groupId param should only be set for tests. public static func localCreateNewGroup(members membersParam: [SignalServiceAddress], groupId: Data? = nil, name: String? = nil, @@ -213,7 +215,8 @@ public class GroupManager: NSObject { builder.addNonPendingMember(localAddress, role: .administrator) return builder.build() }.then(on: .global()) { (groupMembership: GroupMembership) -> Promise in - // Try to obtain profile key credentials for all group members + // If we might create a v2 group, + // try to obtain profile key credentials for all group members // including ourself, unless we already have them on hand. guard RemoteConfig.groupsV2CreateGroups else { return Promise.value(groupMembership) @@ -238,6 +241,14 @@ public class GroupManager: NSObject { throw OWSAssertionError("Missing localAddress.") } + // Build the "initial" group model. + // This will finalize the immutable aspects of the group, e.g.: + // + // * Is it v1 or v2? + // * If it is v2, what is the group secret. + // + // This might not be the "final" model - the + // avatar url path might be filled in below. var builder = TSGroupModelBuilder() builder.groupId = groupId builder.name = name @@ -248,6 +259,27 @@ public class GroupManager: NSObject { return try builder.build(transaction: transaction) } return groupModel + }.then(on: DispatchQueue.global()) { (proposedGroupModel: TSGroupModel) -> Promise in + guard let proposedGroupModelV2 = proposedGroupModel as? TSGroupModelV2 else { + // We don't need to upload avatars for v1 groups. + return Promise.value(proposedGroupModel) + } + guard let avatarData = avatarData else { + // No avatar to upload. + return Promise.value(proposedGroupModel) + } + // Upload avatar. + return firstly { + self.groupsV2.uploadGroupAvatar(avatarData: avatarData, + groupSecretParamsData: proposedGroupModelV2.secretParamsData) + }.map(on: DispatchQueue.global()) { (avatarUrlPath: String) -> TSGroupModel in + // Fill in the avatarUrl on the group model. + return try self.databaseStorage.read { transaction in + var builder = proposedGroupModel.asBuilder + builder.avatarUrlPath = avatarUrlPath + return try builder.build(transaction: transaction) + } + } }.then(on: .global()) { (proposedGroupModel: TSGroupModel) -> Promise in guard proposedGroupModel.groupsVersion == .V2 else { // v1 groups don't need to be created on the service. @@ -279,6 +311,8 @@ public class GroupManager: NSObject { self.profileManager.addThread(toProfileWhitelist: thread) if shouldSendMessage { + assert(thread.groupModel.groupAvatarData == nil) + return sendDurableNewGroupMessage(forThread: thread) .map(on: .global()) { _ in return thread @@ -465,6 +499,7 @@ public class GroupManager: NSObject { // GroupsV2 TODO: Elaborate tests to include admins, pending members, etc. let groupMembership = GroupMembership(v1Members: Set(members)) // GroupsV2 TODO: Let tests specify access levels. + // GroupsV2 TODO: Fill in avatarUrlPath when we test v2 groups. let groupAccess = GroupAccess.allAccess // Use buildGroupModel() to fill in defaults, like it was a new group. @@ -502,6 +537,10 @@ public class GroupManager: NSObject { groupUpdateSourceAddress: SignalServiceAddress?, transaction: SDSAnyWriteTransaction) throws -> UpsertGroupResult { + guard isValidGroupId(groupId, groupsVersion: .V1) else { + throw OWSAssertionError("Invalid group id.") + } + let groupMembership = GroupMembership(v1Members: Set(members)) let groupAccess = GroupAccess.forV1 return try remoteUpsertExistingGroup(groupId: groupId, @@ -576,13 +615,13 @@ public class GroupManager: NSObject { let updateInfo: UpdateInfo do { - updateInfo = try self.updateInfo(groupId: groupId, - name: name, - avatarData: avatarData, - groupMembership: groupMembership, - groupAccess: GroupAccess.forV1, - newDMConfiguration: nil, - transaction: transaction) + updateInfo = try self.updateInfoV1(groupId: groupId, + name: name, + avatarData: avatarData, + groupMembership: groupMembership, + dmConfiguration: nil, + transaction: transaction) + } catch GroupsV2Error.redundantChange { guard let groupThread = TSGroupThread.fetch(groupId: groupId, transaction: transaction) else { throw OWSAssertionError("Missing groupThread.") @@ -623,7 +662,6 @@ public class GroupManager: NSObject { name: name, avatarData: avatarData, groupMembership: groupMembership, - groupAccess: groupAccess, dmConfiguration: dmConfiguration, groupUpdateSourceAddress: groupUpdateSourceAddress) case .V2: @@ -643,18 +681,16 @@ public class GroupManager: NSObject { name: String? = nil, avatarData: Data? = nil, groupMembership: GroupMembership, - groupAccess: GroupAccess, dmConfiguration: OWSDisappearingMessagesConfiguration?, groupUpdateSourceAddress: SignalServiceAddress?) -> Promise { return self.databaseStorage.write(.promise) { (transaction) throws -> UpsertGroupResult in - let updateInfo = try self.updateInfo(groupId: groupId, - name: name, - avatarData: avatarData, - groupMembership: groupMembership, - groupAccess: groupAccess, - newDMConfiguration: dmConfiguration, - transaction: transaction) + let updateInfo = try self.updateInfoV1(groupId: groupId, + name: name, + avatarData: avatarData, + groupMembership: groupMembership, + dmConfiguration: dmConfiguration, + transaction: transaction) let newGroupModel = updateInfo.newGroupModel let upsertGroupResult = try self.tryToUpsertExistingGroupThreadInDatabaseAndCreateInfoMessage(newGroupModel: newGroupModel, newDisappearingMessageToken: dmConfiguration?.asToken, @@ -701,40 +737,69 @@ public class GroupManager: NSObject { return firstly { return self.ensureLocalProfileHasCommitmentIfNecessary() - }.map(on: .global()) { () throws -> (UpdateInfo, GroupsV2ChangeSet) in + }.then(on: DispatchQueue.global()) { () -> Promise in + guard let avatarData = avatarData else { + // No avatar to upload. + return Promise.value(nil) + } + let groupModel = try self.databaseStorage.read { (transaction: SDSAnyReadTransaction) throws -> TSGroupModelV2 in + guard let thread = TSGroupThread.fetch(groupId: groupId, transaction: transaction) else { + throw OWSAssertionError("Thread does not exist.") + } + guard let groupModel = thread.groupModel as? TSGroupModelV2 else { + throw OWSAssertionError("Invalid groupModel") + } + return groupModel + } + if groupModel.groupAvatarData == avatarData && groupModel.groupAvatarUrlPath != nil { + // Skip redundant upload; the avatar hasn't changed. + return Promise.value(groupModel.groupAvatarUrlPath) + } + return firstly { + // Upload avatar. + return self.groupsV2.uploadGroupAvatar(avatarData: avatarData, + groupSecretParamsData: groupModel.secretParamsData) + }.map(on: .global()) { (avatarUrlPath: String) throws -> String? in + // Convert Promise to Promise + return avatarUrlPath + } + }.map(on: .global()) { (avatarUrlPath: String?) throws -> (UpdateInfo, GroupsV2ChangeSet) in return try databaseStorage.read { transaction in - let updateInfo = try self.updateInfo(groupId: groupId, - name: name, - avatarData: avatarData, - groupMembership: groupMembership, - groupAccess: groupAccess, - newDMConfiguration: dmConfiguration, - transaction: transaction) - let changeSet = try groupsV2.buildChangeSet(oldGroupModel: updateInfo.oldGroupModel, - newGroupModel: updateInfo.newGroupModel, - oldDMConfiguration: updateInfo.oldDMConfiguration, - newDMConfiguration: updateInfo.newDMConfiguration, - transaction: transaction) + let updateInfo = try self.updateInfoV2(groupId: groupId, + name: name, + avatarData: avatarData, + avatarUrlPath: avatarUrlPath, + groupMembership: groupMembership, + groupAccess: groupAccess, + newDMConfiguration: dmConfiguration, + transaction: transaction) + let changeSet = try self.groupsV2.buildChangeSet(oldGroupModel: updateInfo.oldGroupModel, + newGroupModel: updateInfo.newGroupModel, + oldDMConfiguration: updateInfo.oldDMConfiguration, + newDMConfiguration: updateInfo.newDMConfiguration, + transaction: transaction) return (updateInfo, changeSet) } }.then(on: .global()) { (_: UpdateInfo, changeSet: GroupsV2ChangeSet) throws -> Promise in - return groupsV2.updateExistingGroupOnService(changeSet: changeSet) + return self.groupsV2.updateExistingGroupOnService(changeSet: changeSet) } // GroupsV2 TODO: Handle redundant change error. } // If dmConfiguration is nil, don't change the disappearing messages configuration. - private static func updateInfo(groupId: Data, - name: String? = nil, - avatarData: Data? = nil, - groupMembership proposedGroupMembership: GroupMembership, - groupAccess: GroupAccess, - newDMConfiguration dmConfiguration: OWSDisappearingMessagesConfiguration?, - transaction: SDSAnyReadTransaction) throws -> UpdateInfo { + private static func updateInfoV1(groupId: Data, + name: String? = nil, + avatarData: Data? = nil, + groupMembership proposedGroupMembership: GroupMembership, + dmConfiguration: OWSDisappearingMessagesConfiguration?, + transaction: SDSAnyReadTransaction) throws -> UpdateInfo { guard let thread = TSGroupThread.fetch(groupId: groupId, transaction: transaction) else { throw OWSAssertionError("Thread does not exist.") } let oldGroupModel = thread.groupModel + guard oldGroupModel.groupsVersion == .V1 else { + throw OWSAssertionError("Invalid groupsVersion.") + } guard let localAddress = tsAccountManager.localAddress else { throw OWSAssertionError("Missing localAddress.") } @@ -742,49 +807,19 @@ public class GroupManager: NSObject { let oldDMConfiguration = OWSDisappearingMessagesConfiguration.fetchOrBuildDefault(with: thread, transaction: transaction) let newDMConfiguration = dmConfiguration ?? oldDMConfiguration - let groupMembership: GroupMembership - if oldGroupModel.groupsVersion == .V1 { - // Always ensure we're a member of any v1 group we're updating. - var builder = proposedGroupMembership.asBuilder - builder.remove(localAddress) - builder.addNonPendingMember(localAddress, role: .normal) - groupMembership = builder.build() - } else { - for address in proposedGroupMembership.allUsers { - guard address.uuid != nil else { - throw OWSAssertionError("Group v2 member missing uuid.") - } - } - // Before we update a v2 group, we need to separate out the - // pending and non-pending members. - groupMembership = self.separatePendingMembers(in: proposedGroupMembership, - oldGroupModel: oldGroupModel, - transaction: transaction) + // Always ensure we're a member of any v1 group we're updating. + var builder = proposedGroupMembership.asBuilder + builder.remove(localAddress) + builder.addNonPendingMember(localAddress, role: .normal) + let groupMembership = builder.build() - guard groupMembership.nonPendingMembers.contains(localAddress) else { - throw OWSAssertionError("Missing localAddress.") - } + var groupModelBuilder = oldGroupModel.asBuilder + groupModelBuilder.name = name + groupModelBuilder.avatarData = avatarData + groupModelBuilder.groupMembership = groupMembership + groupModelBuilder.name = name + let newGroupModel = try groupModelBuilder.build(transaction: transaction) - // Don't try to modify a v2 group if we're not a member. - guard groupMembership.nonPendingMembers.contains(localAddress) else { - throw OWSAssertionError("Missing localAddress.") - } - } - - // GroupsV2 TODO: Eventually we won't need to increment the revision here, - // since we'll probably be updating the TSGroupThread's - // group models with one derived from the service. - let newRevision = oldGroupModel.groupV2Revision + 1 - var builder = TSGroupModelBuilder() - builder.groupId = oldGroupModel.groupId - builder.name = name - builder.avatarData = avatarData - builder.groupMembership = groupMembership - builder.groupAccess = groupAccess - builder.groupsVersion = oldGroupModel.groupsVersion - builder.groupV2Revision = newRevision - builder.groupSecretParamsData = oldGroupModel.groupSecretParamsData - let newGroupModel = try builder.build(transaction: transaction) if oldGroupModel.isEqual(to: newGroupModel) { // Skip redundant update. throw GroupsV2Error.redundantChange @@ -797,6 +832,115 @@ public class GroupManager: NSObject { newDMConfiguration: newDMConfiguration) } + // If dmConfiguration is nil, don't change the disappearing messages configuration. + private static func updateInfoV2(groupId: Data, + name: String? = nil, + avatarData: Data? = nil, + avatarUrlPath: String?, + groupMembership proposedGroupMembership: GroupMembership, + groupAccess: GroupAccess, + newDMConfiguration dmConfiguration: OWSDisappearingMessagesConfiguration?, + transaction: SDSAnyReadTransaction) throws -> UpdateInfo { + + guard let thread = TSGroupThread.fetch(groupId: groupId, transaction: transaction) else { + throw OWSAssertionError("Thread does not exist.") + } + guard let oldGroupModel = thread.groupModel as? TSGroupModelV2 else { + throw OWSAssertionError("Invalid groupModel.") + } + guard let localAddress = tsAccountManager.localAddress else { + throw OWSAssertionError("Missing localAddress.") + } + + let oldDMConfiguration = OWSDisappearingMessagesConfiguration.fetchOrBuildDefault(with: thread, transaction: transaction) + let newDMConfiguration = dmConfiguration ?? oldDMConfiguration + + for address in proposedGroupMembership.allUsers { + guard address.uuid != nil else { + throw OWSAssertionError("Group v2 member missing uuid.") + } + } + // Before we update a v2 group, we need to separate out the + // pending and non-pending members. + let groupMembership = self.separatePendingMembers(in: proposedGroupMembership, + oldGroupModel: oldGroupModel, + transaction: transaction) + + guard groupMembership.nonPendingMembers.contains(localAddress) else { + throw OWSAssertionError("Missing localAddress.") + } + + // Don't try to modify a v2 group if we're not a member. + guard groupMembership.nonPendingMembers.contains(localAddress) else { + throw OWSAssertionError("Missing localAddress.") + } + + let hasAvatarUrlPath = avatarUrlPath != nil + let hasAvatarData = avatarData != nil + guard hasAvatarUrlPath == hasAvatarData else { + throw OWSAssertionError("hasAvatarUrlPath: \(hasAvatarData) != hasAvatarData.") + } + + // GroupsV2 TODO: Eventually we won't need to increment the revision here, + // since we'll probably be updating the TSGroupThread's + // group models with one derived from the service. + let newRevision = oldGroupModel.groupV2Revision + 1 + + var builder = TSGroupModelBuilder() + builder.groupId = oldGroupModel.groupId + builder.name = name + builder.avatarData = avatarData + builder.groupMembership = groupMembership + builder.groupAccess = groupAccess + builder.groupsVersion = oldGroupModel.groupsVersion + builder.groupV2Revision = newRevision + builder.groupSecretParamsData = oldGroupModel.groupSecretParamsData + builder.avatarUrlPath = avatarUrlPath + let newGroupModel = try builder.build(transaction: transaction) + + if oldGroupModel.isEqual(to: newGroupModel) { + // Skip redundant update. + throw GroupsV2Error.redundantChange + } + + return UpdateInfo(groupId: groupId, + oldGroupModel: oldGroupModel, + newGroupModel: newGroupModel, + oldDMConfiguration: oldDMConfiguration, + newDMConfiguration: newDMConfiguration) + } + + @objc + public static func remoteUpdateToExistingGroupV1(groupId: Data, + name: String? = nil, + avatarData: Data? = nil, + groupMembership: GroupMembership, + groupUpdateSourceAddress: SignalServiceAddress?, + transaction: SDSAnyWriteTransaction) throws -> UpsertGroupResult { + + let updateInfo: UpdateInfo + do { + updateInfo = try self.updateInfoV1(groupId: groupId, + name: name, + avatarData: avatarData, + groupMembership: groupMembership, + dmConfiguration: nil, + transaction: transaction) + } catch GroupsV2Error.redundantChange { + guard let groupThread = TSGroupThread.fetch(groupId: groupId, transaction: transaction) else { + throw OWSAssertionError("Missing groupThread.") + } + return UpsertGroupResult(action: .unchanged, groupThread: groupThread) + } + let newGroupModel = updateInfo.newGroupModel + // newDisappearingMessageToken is nil, don't update the disappearing messages configuration. + return try self.tryToUpsertExistingGroupThreadInDatabaseAndCreateInfoMessage(newGroupModel: newGroupModel, + newDisappearingMessageToken: nil, + groupUpdateSourceAddress: groupUpdateSourceAddress, + canInsert: false, + transaction: transaction) + } + // MARK: - Disappearing Messages // This method works with v1 group threads and contact threads. @@ -1008,21 +1152,29 @@ public class GroupManager: NSObject { return databaseStorage.read(.promise) { transaction in let expiresInSeconds = thread.disappearingMessagesDuration(with: transaction) let messageBuilder = TSOutgoingMessageBuilder(thread: thread) - messageBuilder.groupMetaMessage = .update messageBuilder.expiresInSeconds = expiresInSeconds - if FeatureFlags.groupsV2embedProtosInGroupUpdates { - messageBuilder.changeActionsProtoData = changeActionsProtoData + // V2 group update messages mostly ignore groupMetaMessage, + // but we set it to get the right behavior in shouldBeSaved. + // i.e. we need to flag this message as a group update that + // is "durable but transient" - it should not be saved. + messageBuilder.groupMetaMessage = .update + if thread.isGroupV2Thread { + if FeatureFlags.groupsV2embedProtosInGroupUpdates { + messageBuilder.changeActionsProtoData = changeActionsProtoData + } + self.addAdditionalRecipients(to: messageBuilder, + groupThread: thread, + transaction: transaction) } - self.addAdditionalRecipients(to: messageBuilder, - groupThread: thread, - transaction: transaction) return messageBuilder.build() }.then(on: .global()) { (message: TSOutgoingMessage) throws -> Promise in let groupModel = thread.groupModel - if let avatarData = groupModel.groupAvatarData, + // V1 group updates need to include the group avatar (if any) + // as an attachment. + if thread.isGroupV1Thread, + let avatarData = groupModel.groupAvatarData, avatarData.count > 0 { if let dataSource = DataSourceValue.dataSource(with: avatarData, fileExtension: "png") { - // DURABLE CLEANUP - currently one caller uses the completion handler to delete the tappable error message // which causes this code to be called. Once we're more aggressive about durable sending retry, // we could get rid of this "retryable tappable error message". @@ -1047,23 +1199,32 @@ public class GroupManager: NSObject { } private static func sendDurableNewGroupMessage(forThread thread: TSGroupThread) -> Promise { - assert(thread.groupModel.groupAvatarData == nil) - guard !FeatureFlags.groupsV2dontSendUpdates else { return Promise.value(()) } - return databaseStorage.write(.promise) { transaction in - let expiresInSeconds = thread.disappearingMessagesDuration(with: transaction) - let messageBuilder = TSOutgoingMessageBuilder(thread: thread) - messageBuilder.groupMetaMessage = .new - messageBuilder.expiresInSeconds = expiresInSeconds - self.addAdditionalRecipients(to: messageBuilder, - groupThread: thread, - transaction: transaction) - let message = messageBuilder.build() - self.messageSenderJobQueue.add(message: message.asPreparer, - transaction: transaction) + return firstly { + databaseStorage.write(.promise) { transaction in + let expiresInSeconds = thread.disappearingMessagesDuration(with: transaction) + let messageBuilder = TSOutgoingMessageBuilder(thread: thread) + messageBuilder.groupMetaMessage = .new + messageBuilder.expiresInSeconds = expiresInSeconds + self.addAdditionalRecipients(to: messageBuilder, + groupThread: thread, + transaction: transaction) + let message = messageBuilder.build() + self.messageSenderJobQueue.add(message: message.asPreparer, + transaction: transaction) + } + }.then(on: .global()) { _ -> Promise in + // The "new group" update message for v1 groups doesn't support avatars. + // So, if a new v1 group has an avatar, we need to send a group update + // message. + guard thread.groupModel.groupsVersion == .V1, + thread.groupModel.groupAvatarData != nil else { + return Promise.value(()) + } + return self.sendGroupUpdateMessage(thread: thread) } } diff --git a/SignalServiceKit/src/groups/GroupsV2.swift b/SignalServiceKit/src/groups/GroupsV2.swift index 0e6a2ea8c8..668e65aaa4 100644 --- a/SignalServiceKit/src/groups/GroupsV2.swift +++ b/SignalServiceKit/src/groups/GroupsV2.swift @@ -79,6 +79,12 @@ public protocol GroupsV2Swift: GroupsV2 { disappearingMessageToken: DisappearingMessageToken) -> Promise func reuploadLocalProfilePromise() -> Promise + + func updateGroupWithChangeActions(groupId: Data, + changeActionsProto: GroupsProtoGroupChangeActions, + groupSecretParamsData: Data) throws -> Promise + + func uploadGroupAvatar(avatarData: Data, groupSecretParamsData: Data) -> Promise } // MARK: - @@ -87,6 +93,9 @@ public protocol GroupsV2ChangeSet: AnyObject { var groupId: Data { get } var groupSecretParamsData: Data { get } + var newAvatarData: Data? { get } + var newAvatarUrlPath: String? { get } + func buildGroupChangeProto(currentGroupModel: TSGroupModel, currentDisappearingMessageToken: DisappearingMessageToken) -> Promise } @@ -115,10 +124,6 @@ public protocol GroupV2Updates: AnyObject { func tryToRefreshV2GroupUpToSpecificRevisionImmediately(_ groupThread: TSGroupThread, upToRevision: UInt32) - - func updateGroupWithChangeActions(groupId: Data, - changeActionsProto: GroupsProtoGroupChangeActions, - transaction: SDSAnyWriteTransaction) throws -> TSGroupThread } // MARK: - @@ -127,6 +132,11 @@ public protocol GroupV2UpdatesSwift: GroupV2Updates { func tryToRefreshV2GroupThreadWithThrottling(groupId: Data, groupSecretParamsData: Data, groupUpdateMode: GroupUpdateMode) -> Promise + + func updateGroupWithChangeActions(groupId: Data, + changeActionsProto: GroupsProtoGroupChangeActions, + downloadedAvatars: GroupV2DownloadedAvatars, + transaction: SDSAnyWriteTransaction) throws -> TSGroupThread } // MARK: - @@ -141,7 +151,8 @@ public protocol GroupV2Snapshot { var title: String { get } - // GroupsV2 TODO: Avatar. + var avatarUrlPath: String? { get } + var avatarData: Data? { get } var groupMembership: GroupMembership { get } @@ -188,7 +199,72 @@ public class GroupV2ContextInfo: NSObject { // MARK: - -public class MockGroupsV2: NSObject, GroupsV2, GroupsV2Swift { +public struct GroupV2DownloadedAvatars { + // A map of avatar url-to-avatar data. + private var avatarMap = [String: Data]() + + public init() {} + + public mutating func set(avatarData: Data, avatarUrlPath: String) { + avatarMap[avatarUrlPath] = avatarData + } + + public mutating func merge(_ other: GroupV2DownloadedAvatars) { + for (avatarUrlPath, avatarData) in other.avatarMap { + avatarMap[avatarUrlPath] = avatarData + } + } + + public func hasAvatarData(for avatarUrlPath: String) -> Bool { + return avatarMap[avatarUrlPath] != nil + } + + public func avatarData(for avatarUrlPath: String) throws -> Data { + guard let avatarData = avatarMap[avatarUrlPath] else { + throw OWSAssertionError("Missing avatarData.") + } + return avatarData + } + + public var avatarUrlPaths: [String] { + return Array(avatarMap.keys) + } + + public static var empty: GroupV2DownloadedAvatars { + return GroupV2DownloadedAvatars() + } + + public static func from(groupModel: TSGroupModelV2) -> GroupV2DownloadedAvatars { + return from(avatarData: groupModel.groupAvatarData, avatarUrlPath: groupModel.groupAvatarUrlPath) + } + + public static func from(changeSet: GroupsV2ChangeSet) -> GroupV2DownloadedAvatars { + return from(avatarData: changeSet.newAvatarData, avatarUrlPath: changeSet.newAvatarUrlPath) + } + + private static func from(avatarData: Data?, avatarUrlPath: String?) -> GroupV2DownloadedAvatars { + let hasAvatarData = avatarData != nil + let hasAvatarUrlPath = avatarUrlPath != nil + guard hasAvatarData == hasAvatarUrlPath else { + // Fail but continue in production; we can recover from this scenario. + owsFailDebug("hasAvatarData: \(hasAvatarData) != hasAvatarUrlPath: \(hasAvatarUrlPath)") + return .empty + } + guard let avatarData = avatarData, + let avatarUrlPath = avatarUrlPath else { + // No avatar. + return .empty + } + // Avatar found, add it to the result set. + var downloadedAvatars = GroupV2DownloadedAvatars() + downloadedAvatars.set(avatarData: avatarData, avatarUrlPath: avatarUrlPath) + return downloadedAvatars + } +} + +// MARK: - + +public class MockGroupsV2: NSObject, GroupsV2Swift { public func createNewGroupOnService(groupModel: TSGroupModel) -> Promise { owsFail("Not implemented.") @@ -286,6 +362,17 @@ public class MockGroupsV2: NSObject, GroupsV2, GroupsV2Swift { public func updateLocalProfileKeyInGroup(groupId: Data, transaction: SDSAnyWriteTransaction) { owsFail("Not implemented.") } + + public func updateGroupWithChangeActions(groupId: Data, + changeActionsProto: GroupsProtoGroupChangeActions, + groupSecretParamsData: Data) throws -> Promise { + owsFail("Not implemented.") + } + + public func uploadGroupAvatar(avatarData: Data, + groupSecretParamsData: Data) -> Promise { + owsFail("Not implemented.") + } } // MARK: - @@ -310,6 +397,7 @@ public class MockGroupV2Updates: NSObject, GroupV2UpdatesSwift { public func updateGroupWithChangeActions(groupId: Data, changeActionsProto: GroupsProtoGroupChangeActions, + downloadedAvatars: GroupV2DownloadedAvatars, transaction: SDSAnyWriteTransaction) throws -> TSGroupThread { owsFail("Not implemented.") } diff --git a/SignalServiceKit/src/groups/TSGroupModel.h b/SignalServiceKit/src/groups/TSGroupModel.h index 552b39d336..70f88abce1 100644 --- a/SignalServiceKit/src/groups/TSGroupModel.h +++ b/SignalServiceKit/src/groups/TSGroupModel.h @@ -45,9 +45,6 @@ typedef NS_CLOSED_ENUM( @property (nonatomic, readonly) uint32_t groupV2Revision; @property (nonatomic, readonly, nullable) NSData *groupSecretParamsData; -// GroupsV2 TODO: This should be done via GroupManager. -- (void)setGroupAvatarDataWithImage:(nullable UIImage *)image; - + (nullable NSData *)dataForGroupAvatar:(nullable UIImage *)image; - (instancetype)init NS_UNAVAILABLE; diff --git a/SignalServiceKit/src/groups/TSGroupModel.m b/SignalServiceKit/src/groups/TSGroupModel.m index 716c0a782a..7567df357f 100644 --- a/SignalServiceKit/src/groups/TSGroupModel.m +++ b/SignalServiceKit/src/groups/TSGroupModel.m @@ -148,11 +148,6 @@ NSUInteger const TSGroupModelSchemaVersion = 1; return data; } -- (void)setGroupAvatarDataWithImage:(nullable UIImage *)image -{ - self.groupAvatarData = [TSGroupModel dataForGroupAvatar:image]; -} - - (nullable UIImage *)groupAvatarImage { return [UIImage imageWithData:self.groupAvatarData]; diff --git a/SignalServiceKit/src/groups/TSGroupModel.swift b/SignalServiceKit/src/groups/TSGroupModel.swift index f87669f057..24e009b836 100644 --- a/SignalServiceKit/src/groups/TSGroupModel.swift +++ b/SignalServiceKit/src/groups/TSGroupModel.swift @@ -32,11 +32,13 @@ public class TSGroupModelV2: TSGroupModel { @objc var membership: GroupMembership = GroupMembership.empty @objc - var access: GroupAccess = GroupAccess.defaultForV2 + var access: GroupAccess = .defaultV2Access @objc var secretParamsData: Data = Data() @objc var revision: UInt32 = 0 + @objc + var avatarUrlPath: String? @objc public required init(groupId: Data, @@ -45,13 +47,15 @@ public class TSGroupModelV2: TSGroupModel { groupMembership: GroupMembership, groupAccess: GroupAccess, revision: UInt32, - secretParamsData: Data) { + secretParamsData: Data, + avatarUrlPath: String?) { assert(secretParamsData.count > 0) self.membership = groupMembership self.secretParamsData = secretParamsData self.access = groupAccess self.revision = revision + self.avatarUrlPath = avatarUrlPath super.init(groupId: groupId, name: name, @@ -103,6 +107,11 @@ public class TSGroupModelV2: TSGroupModel { return secretParamsData } + @objc + public var groupAvatarUrlPath: String? { + return avatarUrlPath + } + @objc public override func isEqual(to model: TSGroupModel) -> Bool { guard super.isEqual(to: model) else { @@ -123,6 +132,9 @@ public class TSGroupModelV2: TSGroupModel { guard other.revision == revision else { return false } + guard other.avatarUrlPath == avatarUrlPath else { + return false + } return true } @@ -132,11 +144,12 @@ public class TSGroupModelV2: TSGroupModel { result += "groupId: \(groupId.hexadecimalString),\n" result += "groupsVersion: \(groupsVersion),\n" result += "groupName: \(String(describing: groupName)),\n" - result += "groupAvatarData: \(String(describing: groupAvatarData?.hexadecimalString)),\n" + result += "groupAvatarData: \(String(describing: groupAvatarData?.hexadecimalString.prefix(256))),\n" result += "membership: \(groupMembership.debugDescription),\n" result += "groupAccess: \(groupAccess.debugDescription),\n" result += "groupSecretParamsData: \(secretParamsData.hexadecimalString),\n" result += "revision: \(revision),\n" + result += "avatarUrlPath: \(String(describing: avatarUrlPath)),\n" result += "]" return result } diff --git a/SignalServiceKit/src/groups/TSGroupModelBuilder.swift b/SignalServiceKit/src/groups/TSGroupModelBuilder.swift index 411e66c4b6..e81a79c69d 100644 --- a/SignalServiceKit/src/groups/TSGroupModelBuilder.swift +++ b/SignalServiceKit/src/groups/TSGroupModelBuilder.swift @@ -23,6 +23,7 @@ public struct TSGroupModelBuilder { public var groupV2Revision: UInt32 = 0 public var groupSecretParamsData: Data? public var newGroupSeed: NewGroupSeed? + public var avatarUrlPath: String? public init() {} @@ -31,13 +32,13 @@ public struct TSGroupModelBuilder { public init(groupV2Snapshot: GroupV2Snapshot) throws { self.groupId = try groupsV2.groupId(forGroupSecretParamsData: groupV2Snapshot.groupSecretParamsData) self.name = groupV2Snapshot.title - // GroupsV2 TODO: Avatar. - // self.avatarData = groupV2Snapshot.avatarData + self.avatarData = groupV2Snapshot.avatarData self.groupMembership = groupV2Snapshot.groupMembership self.groupAccess = groupV2Snapshot.groupAccess self.groupsVersion = GroupsVersion.V2 self.groupV2Revision = groupV2Snapshot.revision self.groupSecretParamsData = groupV2Snapshot.groupSecretParamsData + self.avatarUrlPath = groupV2Snapshot.avatarUrlPath } public func build(transaction: SDSAnyReadTransaction) throws -> TSGroupModel { @@ -76,6 +77,8 @@ public struct TSGroupModelBuilder { case .V2: let groupAccess = buildGroupAccess(groupsVersion: groupsVersion) let groupSecretParamsData = try buildGroupSecretParamsData(newGroupSeed: newGroupSeed) + // Don't set avatarUrlPath unless we have avatarData. + let avatarUrlPath = avatarData != nil ? self.avatarUrlPath : nil return TSGroupModelV2(groupId: groupId, name: name, @@ -83,7 +86,8 @@ public struct TSGroupModelBuilder { groupMembership: groupMembership, groupAccess: groupAccess, revision: groupV2Revision, - secretParamsData: groupSecretParamsData) + secretParamsData: groupSecretParamsData, + avatarUrlPath: avatarUrlPath) } } From 703de8c584fd0b19b6a752ab9cf236ff88940495 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 26 Feb 2020 15:36:40 -0300 Subject: [PATCH 2/4] Fix obsolete assert. --- SignalServiceKit/src/groups/GroupManager.swift | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/SignalServiceKit/src/groups/GroupManager.swift b/SignalServiceKit/src/groups/GroupManager.swift index 612c9de969..3011ef05a1 100644 --- a/SignalServiceKit/src/groups/GroupManager.swift +++ b/SignalServiceKit/src/groups/GroupManager.swift @@ -311,11 +311,10 @@ public class GroupManager: NSObject { self.profileManager.addThread(toProfileWhitelist: thread) if shouldSendMessage { - assert(thread.groupModel.groupAvatarData == nil) - - return sendDurableNewGroupMessage(forThread: thread) - .map(on: .global()) { _ in - return thread + return firstly { + sendDurableNewGroupMessage(forThread: thread) + }.map(on: .global()) { _ in + return thread } } else { return Promise.value(thread) From f2a07f1f99b7e6b206ccafae342d665a612ece51 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 26 Feb 2020 16:38:19 -0300 Subject: [PATCH 3/4] Respond to CR. --- .../groups/GroupsV2ChangeSetImpl.swift | 3 +- SignalMessaging/groups/GroupsV2Impl.swift | 38 +++++++++++++------ SignalMessaging/groups/GroupsV2Protos.swift | 16 +++++--- .../src/Network/API/OWSUploadV2.m | 1 - .../Receiving/GroupsV2MessageProcessor.swift | 5 ++- .../src/groups/GroupManager.swift | 31 +-------------- SignalServiceKit/src/groups/GroupsV2.swift | 2 + 7 files changed, 46 insertions(+), 50 deletions(-) diff --git a/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift b/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift index 45ef0ee424..74ac311fcd 100644 --- a/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift +++ b/SignalMessaging/groups/GroupsV2ChangeSetImpl.swift @@ -125,8 +125,9 @@ public class GroupsV2ChangeSetImpl: NSObject, GroupsV2ChangeSet { } // GroupsV2 TODO: Will production implementation of encryptString() pad? + let oldTitle = newGroupModel.groupName?.stripped ?? " " let newTitle = newGroupModel.groupName?.stripped ?? " " - if oldGroupModel.groupName != newTitle { + if oldTitle != newTitle { setTitle(newTitle) } diff --git a/SignalMessaging/groups/GroupsV2Impl.swift b/SignalMessaging/groups/GroupsV2Impl.swift index 27065aeec2..331532d749 100644 --- a/SignalMessaging/groups/GroupsV2Impl.swift +++ b/SignalMessaging/groups/GroupsV2Impl.swift @@ -186,9 +186,11 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { // we should use exactly the same group model that was used to construct // the update request - which should reflect pre-update service state. return firstly { + // We can ignoreSignature because these protos came from the service. return self.updateGroupWithChangeActions(groupId: groupId, changeActionsProto: changeActionsProto, justUploadedAvatars: downloadedAvatars, + ignoreSignature: true, groupV2Params: groupV2Params) }.map(on: DispatchQueue.global()) { (groupThread: TSGroupThread) -> UpdatedV2Group in return UpdatedV2Group(groupThread: groupThread, changeActionsProtoData: changeActionsProtoData) @@ -231,31 +233,37 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { public func updateGroupWithChangeActions(groupId: Data, changeActionsProto: GroupsProtoGroupChangeActions, + ignoreSignature: Bool, groupSecretParamsData: Data) throws -> Promise { let groupV2Params = try GroupV2Params(groupSecretParamsData: groupSecretParamsData) return updateGroupWithChangeActions(groupId: groupId, changeActionsProto: changeActionsProto, justUploadedAvatars: nil, + ignoreSignature: ignoreSignature, groupV2Params: groupV2Params) } public func updateGroupWithChangeActions(groupId: Data, changeActionsProto: GroupsProtoGroupChangeActions, + ignoreSignature: Bool, groupV2Params: GroupV2Params) -> Promise { return updateGroupWithChangeActions(groupId: groupId, changeActionsProto: changeActionsProto, justUploadedAvatars: nil, + ignoreSignature: ignoreSignature, groupV2Params: groupV2Params) } private func updateGroupWithChangeActions(groupId: Data, - changeActionsProto: GroupsProtoGroupChangeActions, - justUploadedAvatars: GroupV2DownloadedAvatars?, - groupV2Params: GroupV2Params) -> Promise { + changeActionsProto: GroupsProtoGroupChangeActions, + justUploadedAvatars: GroupV2DownloadedAvatars?, + ignoreSignature: Bool, + groupV2Params: GroupV2Params) -> Promise { return firstly { self.fetchAllAvatarData(changeActionsProto: changeActionsProto, justUploadedAvatars: justUploadedAvatars, + ignoreSignature: ignoreSignature, groupV2Params: groupV2Params) }.map(on: DispatchQueue.global()) { (downloadedAvatars: GroupV2DownloadedAvatars) -> TSGroupThread in return try self.databaseStorage.write { transaction in @@ -326,7 +334,7 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { public func fetchCurrentGroupV2Snapshot(groupSecretParamsData: Data) -> Promise { return fetchCurrentGroupV2Snapshot(groupSecretParamsData: groupSecretParamsData, - justUploadedAvatars: nil) + justUploadedAvatars: nil) } private func fetchCurrentGroupV2Snapshot(groupSecretParamsData: Data, @@ -363,7 +371,11 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return try GroupsProtoGroup.parseData(groupProtoData) }.then(on: DispatchQueue.global()) { (groupProto: GroupsProtoGroup) -> Promise<(GroupsProtoGroup, GroupV2DownloadedAvatars)> in return firstly { - self.fetchAllAvatarData(groupProto: groupProto, justUploadedAvatars: justUploadedAvatars, groupV2Params: groupV2Params) + // We can ignoreSignature; these protos came from the service. + self.fetchAllAvatarData(groupProto: groupProto, + justUploadedAvatars: justUploadedAvatars, + ignoreSignature: true, + groupV2Params: groupV2Params) }.map(on: DispatchQueue.global()) { (downloadedAvatars: GroupV2DownloadedAvatars) -> (GroupsProtoGroup, GroupV2DownloadedAvatars) in return (groupProto, downloadedAvatars) } @@ -429,11 +441,14 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return try GroupsProtoGroupChanges.parseData(groupChangesProtoData) }.then(on: DispatchQueue.global()) { (groupChangesProto: GroupsProtoGroupChanges) -> Promise<[GroupV2Change]> in return firstly { - self.fetchAllAvatarData(groupChangesProto: groupChangesProto, groupV2Params: groupV2Params) + // We can ignoreSignature; these protos came from the service. + self.fetchAllAvatarData(groupChangesProto: groupChangesProto, + ignoreSignature: true, + groupV2Params: groupV2Params) }.map(on: DispatchQueue.global()) { (downloadedAvatars: GroupV2DownloadedAvatars) -> [GroupV2Change] in - try GroupsV2Protos.parse(groupChangesProto: groupChangesProto, - downloadedAvatars: downloadedAvatars, - groupV2Params: groupV2Params) + try GroupsV2Protos.parseChangesFromService(groupChangesProto: groupChangesProto, + downloadedAvatars: downloadedAvatars, + groupV2Params: groupV2Params) } } } @@ -450,6 +465,7 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { groupChangesProto: GroupsProtoGroupChanges? = nil, changeActionsProto: GroupsProtoGroupChangeActions? = nil, justUploadedAvatars: GroupV2DownloadedAvatars? = nil, + ignoreSignature: Bool, groupV2Params: GroupV2Params) -> Promise { var downloadedAvatars = GroupV2DownloadedAvatars() @@ -470,18 +486,18 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return TSGroupThread.fetch(groupId: groupId, transaction: transaction) }) else { // Thread doesn't exist in database yet. - return () + return } guard let groupModel = groupThread.groupModel as? TSGroupModelV2 else { throw OWSAssertionError("Invalid groupModel.") } // Try to add avatar from group model, if any. downloadedAvatars.merge(GroupV2DownloadedAvatars.from(groupModel: groupModel)) - return () }.then(on: DispatchQueue.global()) { () -> Promise<[String]> in GroupsV2Protos.collectAvatarUrlPaths(groupProto: groupProto, groupChangesProto: groupChangesProto, changeActionsProto: changeActionsProto, + ignoreSignature: ignoreSignature, groupV2Params: groupV2Params) }.then(on: DispatchQueue.global()) { (protoAvatarUrlPaths: [String]) -> Promise in diff --git a/SignalMessaging/groups/GroupsV2Protos.swift b/SignalMessaging/groups/GroupsV2Protos.swift index 43448e314b..3910548b0f 100644 --- a/SignalMessaging/groups/GroupsV2Protos.swift +++ b/SignalMessaging/groups/GroupsV2Protos.swift @@ -330,9 +330,9 @@ public class GroupsV2Protos { // MARK: - // We do not treat an empty response with no changes as an error. - public class func parse(groupChangesProto: GroupsProtoGroupChanges, - downloadedAvatars: GroupV2DownloadedAvatars, - groupV2Params: GroupV2Params) throws -> [GroupV2Change] { + public class func parseChangesFromService(groupChangesProto: GroupsProtoGroupChanges, + downloadedAvatars: GroupV2DownloadedAvatars, + groupV2Params: GroupV2Params) throws -> [GroupV2Change] { var result = [GroupV2Change]() for changeStateProto in groupChangesProto.groupChanges { guard let snapshotProto = changeStateProto.groupState else { @@ -344,6 +344,7 @@ public class GroupsV2Protos { guard let changeProto = changeStateProto.groupChange else { throw OWSAssertionError("Missing groupChange proto.") } + // We can ignoreSignature because these protos came from the service. let changeActionsProto: GroupsProtoGroupChangeActions = try parseAndVerifyChangeActionsProto(changeProto, ignoreSignature: true) result.append(GroupV2Change(snapshot: snapshot, changeActionsProto: changeActionsProto)) } @@ -355,6 +356,7 @@ public class GroupsV2Protos { public class func collectAvatarUrlPaths(groupProto: GroupsProtoGroup? = nil, groupChangesProto: GroupsProtoGroupChanges? = nil, changeActionsProto: GroupsProtoGroupChangeActions? = nil, + ignoreSignature: Bool, groupV2Params: GroupV2Params) -> Promise<[String]> { return DispatchQueue.global().async(.promise) { () throws -> [String] in var avatarUrlPaths = [String]() @@ -363,7 +365,8 @@ public class GroupsV2Protos { } if let groupChangesProto = groupChangesProto { avatarUrlPaths += try self.collectAvatarUrlPaths(groupChangesProto: groupChangesProto, - groupV2Params: groupV2Params) + ignoreSignature: ignoreSignature, + groupV2Params: groupV2Params) } if let changeActionsProto = changeActionsProto { avatarUrlPaths += self.collectAvatarUrlPaths(changeActionsProto: changeActionsProto) @@ -373,7 +376,7 @@ public class GroupsV2Protos { } } - private class func collectAvatarUrlPaths(groupChangesProto: GroupsProtoGroupChanges, + private class func collectAvatarUrlPaths(groupChangesProto: GroupsProtoGroupChanges, ignoreSignature: Bool, groupV2Params: GroupV2Params) throws -> [String] { var avatarUrlPaths = [String]() for changeStateProto in groupChangesProto.groupChanges { @@ -385,7 +388,8 @@ public class GroupsV2Protos { guard let changeProto = changeStateProto.groupChange else { throw OWSAssertionError("Missing groupChange proto.") } - let changeActionsProto = try parseAndVerifyChangeActionsProto(changeProto, ignoreSignature: true) + // We can ignoreSignature because these protos came from the service. + let changeActionsProto = try parseAndVerifyChangeActionsProto(changeProto, ignoreSignature: ignoreSignature) avatarUrlPaths += self.collectAvatarUrlPaths(changeActionsProto: changeActionsProto) } return avatarUrlPaths diff --git a/SignalServiceKit/src/Network/API/OWSUploadV2.m b/SignalServiceKit/src/Network/API/OWSUploadV2.m index bb542a0660..92b4916e45 100644 --- a/SignalServiceKit/src/Network/API/OWSUploadV2.m +++ b/SignalServiceKit/src/Network/API/OWSUploadV2.m @@ -193,7 +193,6 @@ void AppendMultipartFormPath(id formData, NSString *name, N return resolve(@(1)); } - // TODO: Should we use a non-empty urlPath? [[strongSelf parseFormAndUpload:formResponseObject progressBlock:progressBlock] .thenInBackground(^{ return resolve(@(1)); diff --git a/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift b/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift index a92c0646ab..3e0f78aef9 100644 --- a/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift +++ b/SignalServiceKit/src/Network/Receiving/GroupsV2MessageProcessor.swift @@ -547,7 +547,7 @@ class IncomingGroupsV2MessageQueue: NSObject { } DispatchQueue.global().async(.promise) { - // We need to verify the signature because this proto came from + // We need to verify the signatures because these protos came from // another client, not the service. return try self.groupsV2.parseAndVerifyChangeActionsProto(changeActionsProtoData, ignoreSignature: false) @@ -555,8 +555,11 @@ class IncomingGroupsV2MessageQueue: NSObject { guard let groupSecretParamsData = oldGroupModel.groupSecretParamsData else { throw OWSAssertionError("Missing groupSecretParamsData.") } + // We need to verify the signatures because these protos came from + // another client, not the service. return try self.groupsV2.updateGroupWithChangeActions(groupId: oldGroupModel.groupId, changeActionsProto: changeActionsProto, + ignoreSignature: false, groupSecretParamsData: groupSecretParamsData) }.map(on: .global()) { (updatedGroupThread: TSGroupThread) throws -> Void in let updatedGroupModel = updatedGroupThread.groupModel diff --git a/SignalServiceKit/src/groups/GroupManager.swift b/SignalServiceKit/src/groups/GroupManager.swift index 3011ef05a1..212986b4a6 100644 --- a/SignalServiceKit/src/groups/GroupManager.swift +++ b/SignalServiceKit/src/groups/GroupManager.swift @@ -541,42 +541,13 @@ public class GroupManager: NSObject { } let groupMembership = GroupMembership(v1Members: Set(members)) - let groupAccess = GroupAccess.forV1 - return try remoteUpsertExistingGroup(groupId: groupId, - name: name, - avatarData: avatarData, - groupMembership: groupMembership, - groupAccess: groupAccess, - groupsVersion: .V1, - groupV2Revision: 0, - groupSecretParamsData: nil, - disappearingMessageToken: disappearingMessageToken, - groupUpdateSourceAddress: groupUpdateSourceAddress, - transaction: transaction) - } - - // If disappearingMessageToken is nil, don't update the disappearing messages configuration. - public static func remoteUpsertExistingGroup(groupId: Data, - name: String? = nil, - avatarData: Data? = nil, - groupMembership: GroupMembership, - groupAccess: GroupAccess, - groupsVersion: GroupsVersion, - groupV2Revision: UInt32, - groupSecretParamsData: Data? = nil, - disappearingMessageToken: DisappearingMessageToken?, - groupUpdateSourceAddress: SignalServiceAddress?, - transaction: SDSAnyWriteTransaction) throws -> UpsertGroupResult { var builder = TSGroupModelBuilder() builder.groupId = groupId builder.name = name builder.avatarData = avatarData builder.groupMembership = groupMembership - builder.groupAccess = groupAccess - builder.groupsVersion = groupsVersion - builder.groupV2Revision = groupV2Revision - builder.groupSecretParamsData = groupSecretParamsData + builder.groupsVersion = .V1 let groupModel = try builder.build(transaction: transaction) return try remoteUpsertExistingGroup(groupModel: groupModel, diff --git a/SignalServiceKit/src/groups/GroupsV2.swift b/SignalServiceKit/src/groups/GroupsV2.swift index 668e65aaa4..b0c73031d8 100644 --- a/SignalServiceKit/src/groups/GroupsV2.swift +++ b/SignalServiceKit/src/groups/GroupsV2.swift @@ -82,6 +82,7 @@ public protocol GroupsV2Swift: GroupsV2 { func updateGroupWithChangeActions(groupId: Data, changeActionsProto: GroupsProtoGroupChangeActions, + ignoreSignature: Bool, groupSecretParamsData: Data) throws -> Promise func uploadGroupAvatar(avatarData: Data, groupSecretParamsData: Data) -> Promise @@ -365,6 +366,7 @@ public class MockGroupsV2: NSObject, GroupsV2Swift { public func updateGroupWithChangeActions(groupId: Data, changeActionsProto: GroupsProtoGroupChangeActions, + ignoreSignature: Bool, groupSecretParamsData: Data) throws -> Promise { owsFail("Not implemented.") } From a5ae88e151af5568dd0d411007f6701d782b360a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 26 Feb 2020 16:40:01 -0300 Subject: [PATCH 4/4] Update Cocoapods. --- Pods | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pods b/Pods index 76494d3998..20f8ff17cc 160000 --- a/Pods +++ b/Pods @@ -1 +1 @@ -Subproject commit 76494d39988450506347468043aaee0451a8357d +Subproject commit 20f8ff17cc8ef94ff3580e6421bdc8708c7fde13