diff --git a/SignalMessaging/environment/AppSetup.swift b/SignalMessaging/environment/AppSetup.swift index cfffb172e3..81b158985c 100644 --- a/SignalMessaging/environment/AppSetup.swift +++ b/SignalMessaging/environment/AppSetup.swift @@ -148,6 +148,7 @@ public class AppSetup { recipientFetcher: dependenciesBridge.recipientFetcher, recipientMerger: dependenciesBridge.recipientMerger, tsAccountManager: tsAccountManager, + udManager: udManager, websocketFactory: webSocketFactory ) let messageSendLog = MessageSendLog( diff --git a/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryManager.swift b/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryManager.swift index e721eb6aaa..6435792ad3 100644 --- a/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryManager.swift +++ b/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryManager.swift @@ -104,6 +104,7 @@ public final class ContactDiscoveryManagerImpl: NSObject, ContactDiscoveryManage recipientFetcher: RecipientFetcher, recipientMerger: RecipientMerger, tsAccountManager: TSAccountManager, + udManager: OWSUDManager, websocketFactory: WebSocketFactory ) { self.init( @@ -112,6 +113,7 @@ public final class ContactDiscoveryManagerImpl: NSObject, ContactDiscoveryManage recipientFetcher: recipientFetcher, recipientMerger: recipientMerger, tsAccountManager: tsAccountManager, + udManager: udManager, websocketFactory: websocketFactory ) ) diff --git a/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryTask.swift b/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryTask.swift index c52393d643..33937a9974 100644 --- a/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryTask.swift +++ b/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryTask.swift @@ -16,6 +16,7 @@ final class ContactDiscoveryTaskQueueImpl: ContactDiscoveryTaskQueue { private let recipientFetcher: RecipientFetcher private let recipientMerger: RecipientMerger private let tsAccountManager: TSAccountManager + private let udManager: OWSUDManager private let websocketFactory: WebSocketFactory init( @@ -23,12 +24,14 @@ final class ContactDiscoveryTaskQueueImpl: ContactDiscoveryTaskQueue { recipientFetcher: RecipientFetcher, recipientMerger: RecipientMerger, tsAccountManager: TSAccountManager, + udManager: OWSUDManager, websocketFactory: WebSocketFactory ) { self.db = db self.recipientFetcher = recipientFetcher self.recipientMerger = recipientMerger self.tsAccountManager = tsAccountManager + self.udManager = udManager self.websocketFactory = websocketFactory } @@ -46,41 +49,29 @@ final class ContactDiscoveryTaskQueueImpl: ContactDiscoveryTaskQueue { ) return firstly { - Self.createContactDiscoveryOperation( - for: e164s, + ContactDiscoveryV2Operation( + e164sToLookup: e164s, mode: mode, + tryToReturnAcisWithoutUaks: RemoteConfig.tryToReturnAcisWithoutUaks, + udManager: ContactDiscoveryV2Operation.Wrappers.UDManager(db: db, udManager: udManager), websocketFactory: websocketFactory ).perform(on: workQueue) - }.map(on: workQueue) { (discoveredContacts: Set) -> Set in - try self.processResults(requestedPhoneNumbers: e164s, discoveryResults: discoveredContacts) + }.map(on: workQueue) { (discoveryResults: [ContactDiscoveryV2Operation.DiscoveryResult]) -> Set in + try self.processResults(requestedPhoneNumbers: e164s, discoveryResults: discoveryResults) } } - private static func createContactDiscoveryOperation( - for e164s: Set, - mode: ContactDiscoveryMode, - websocketFactory: WebSocketFactory - ) -> ContactDiscoveryOperation { - return ContactDiscoveryV2CompatibilityOperation( - e164sToLookup: e164s, - mode: mode, - websocketFactory: websocketFactory - ) - } - private func processResults( requestedPhoneNumbers: Set, - discoveryResults: Set + discoveryResults: [ContactDiscoveryV2Operation.DiscoveryResult] ) throws -> Set { - let undiscoverableE164s = requestedPhoneNumbers.subtracting(discoveryResults.lazy.map { $0.e164 }) - return try db.write { tx in guard let localIdentifiers = tsAccountManager.localIdentifiers(transaction: SDSDB.shimOnlyBridge(tx)) else { throw OWSAssertionError("Not registered.") } return storeResults( - discoveredContacts: discoveryResults, - undiscoverableE164s: undiscoverableE164s, + requestedPhoneNumbers: requestedPhoneNumbers, + discoveryResults: discoveryResults, localIdentifiers: localIdentifiers, tx: tx ) @@ -88,45 +79,56 @@ final class ContactDiscoveryTaskQueueImpl: ContactDiscoveryTaskQueue { } private func storeResults( - discoveredContacts: Set, - undiscoverableE164s: Set, + requestedPhoneNumbers: Set, + discoveryResults: [ContactDiscoveryV2Operation.DiscoveryResult], localIdentifiers: LocalIdentifiers, tx: DBWriteTransaction ) -> Set { - let registeredRecipients = Set(discoveredContacts.map { discoveredContact -> SignalRecipient in + var registeredRecipients = Set() + for discoveryResult in discoveryResults { + // PNI TODO: Pass the PNI into the merging logic. + guard let aci = discoveryResult.aci else { + continue + } let recipient = recipientMerger.applyMergeFromContactDiscovery( localIdentifiers: localIdentifiers, - aci: UntypedServiceId(discoveredContact.uuid), - phoneNumber: discoveredContact.e164, + aci: aci.untypedServiceId, + phoneNumber: discoveryResult.e164, tx: tx ) recipient.markAsRegisteredAndSave(tx: SDSDB.shimOnlyBridge(tx)) - return recipient - }) - for undiscoverableE164 in undiscoverableE164s { - let address = SignalServiceAddress(undiscoverableE164) + // We process all the results that we were provided, but we only return the + // recipients that were specifically requested as part of this operation. + if requestedPhoneNumbers.contains(discoveryResult.e164) { + registeredRecipients.insert(recipient) + } + } - // It's possible we have an undiscoverable address that has a UUID in a - // number of scenarios, such as (but not exclusive to) the following: + let undiscoverablePhoneNumbers = requestedPhoneNumbers.subtracting(discoveryResults.lazy.map { $0.e164 }) + for phoneNumber in undiscoverablePhoneNumbers { + // It's possible we have an undiscoverable phone number that already has an + // ACI or PNI in a number of scenarios, such as (but not exclusive to) the + // following: // // * You do "find by phone number" for someone you've previously interacted - // with (and had a UUID for) who is no longer registered. + // with (and had an ACI or PNI for) who is no longer registered. // // * You do an intersection to look up someone who has shared their phone - // number with you (via message send) but has chosen to be undiscoverable - // by CDS lookups. + // number with you (via message send) but has chosen to be undiscoverable + // by CDS lookups. // // When any of these scenarios occur, we cannot know with certainty if the // user is unregistered or has only turned off discoverability, so we // *only* mark the addresses without any UUIDs as unregistered. Everything // else we ignore; we will identify their current registration status // either when attempting to send a message or when fetching their profile. - guard address.uuid == nil else { + let finder = AnySignalRecipientFinder() + let recipient = finder.signalRecipientForPhoneNumber(phoneNumber.stringValue, transaction: SDSDB.shimOnlyBridge(tx)) + // PNI TODO: Also check for PNIs here. + guard let recipient, recipient.serviceId == nil else { continue } - - let recipient = recipientFetcher.fetchOrCreate(phoneNumber: undiscoverableE164, tx: tx) recipient.markAsUnregisteredAndSave(tx: SDSDB.shimOnlyBridge(tx)) } diff --git a/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryV2Operation.swift b/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryV2Operation.swift index 0006cbff84..0dbe7c2436 100644 --- a/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryV2Operation.swift +++ b/SignalServiceKit/src/Contacts/Discovery/ContactDiscoveryV2Operation.swift @@ -5,6 +5,8 @@ import GRDB import Foundation +import LibSignalClient +import SignalCoreKit // MARK: - @@ -14,49 +16,6 @@ private enum Constant { // MARK: - -/// Runs a CDSv1-compatible operation against the CDSv2 backend. -final class ContactDiscoveryV2CompatibilityOperation: ContactDiscoveryOperation { - let e164sToLookup: Set - let mode: ContactDiscoveryMode - let websocketFactory: WebSocketFactory - - init(e164sToLookup: Set, mode: ContactDiscoveryMode, websocketFactory: WebSocketFactory) { - self.e164sToLookup = e164sToLookup - self.mode = mode - self.websocketFactory = websocketFactory - } - - func perform(on queue: DispatchQueue) -> Promise> { - let operation = ContactDiscoveryV2Operation( - e164sToLookup: e164sToLookup, - mode: mode, - compatibilityMode: .fetchAllACIs, - websocketFactory: websocketFactory - ) - return firstly { - operation.perform(on: queue) - }.map(on: queue) { discoveryResults in - var results = Set() - for discoveryResult in discoveryResults { - guard self.e164sToLookup.contains(discoveryResult.e164) else { - // In v2, we get back results for previous lookups as well. The API - // contract expects only those that were explicitly requested, so filter to - // include only those. - continue - } - guard let aci = discoveryResult.aci else { - owsFailDebug("CDSv2: All discovery results should have an ACI") - continue - } - results.insert(DiscoveredContactInfo(e164: discoveryResult.e164, uuid: aci)) - } - return results - } - } -} - -// MARK: - - protocol ContactDiscoveryV2PersistentState { /// Load the token & e164s represented by the token. /// @@ -86,14 +45,7 @@ final class ContactDiscoveryV2Operation { let e164sToLookup: Set - let compatibilityMode: CompatibilityMode - enum CompatibilityMode { - /// Send a request that returns a CDSv1-equivalent response. - case fetchAllACIs - - /// Send a PNP-aware request. Requires full support for PNI-only contacts. - case fetchKnownACIs - } + let tryToReturnAcisWithoutUaks: Bool /// If non-nil, requests will include prevE164s & a token, so we'll only /// consume quota for new E164s. @@ -104,24 +56,29 @@ final class ContactDiscoveryV2Operation { /// consume too much quota without the user's consent. let persistentState: ContactDiscoveryV2PersistentState? + let udManager: Shims.UDManager + let connectionFactory: SgxWebsocketConnectionFactory init( e164sToLookup: Set, - compatibilityMode: CompatibilityMode, + tryToReturnAcisWithoutUaks: Bool, persistentState: ContactDiscoveryV2PersistentState?, + udManager: Shims.UDManager, connectionFactory: SgxWebsocketConnectionFactory ) { self.e164sToLookup = e164sToLookup - self.compatibilityMode = compatibilityMode + self.tryToReturnAcisWithoutUaks = tryToReturnAcisWithoutUaks self.persistentState = persistentState + self.udManager = udManager self.connectionFactory = connectionFactory } convenience init( e164sToLookup: Set, mode: ContactDiscoveryMode, - compatibilityMode: CompatibilityMode, + tryToReturnAcisWithoutUaks: Bool, + udManager: Shims.UDManager, websocketFactory: WebSocketFactory ) { let persistentState: ContactDiscoveryV2PersistentState? @@ -132,8 +89,9 @@ final class ContactDiscoveryV2Operation { } self.init( e164sToLookup: e164sToLookup, - compatibilityMode: compatibilityMode, + tryToReturnAcisWithoutUaks: tryToReturnAcisWithoutUaks, persistentState: persistentState, + udManager: udManager, connectionFactory: SgxWebsocketConnectionFactoryImpl(websocketFactory: websocketFactory) ) } @@ -207,14 +165,15 @@ final class ContactDiscoveryV2Operation { request.token = prevToken request.prevE164S = prevE164s request.newE164S = ContactDiscoveryE164Collection(newE164s).encodedValues - - switch compatibilityMode { - case .fetchAllACIs: - request.returnAcisWithoutUaks = true - case .fetchKnownACIs: - request.returnAcisWithoutUaks = false - // TODO: Fetch all ACI-UAK pairs. - } + request.returnAcisWithoutUaks = tryToReturnAcisWithoutUaks + request.aciUakPairs = { () -> Data in + var result = Data() + for (aci, uak) in udManager.fetchAllAciUakPairsWithSneakyTransaction() { + result.append(contentsOf: aci.wrappedAciValue.serviceIdBinary) + result.append(uak.keyData) + } + return result + }() return InitialRequest( hasToken: !prevToken.isEmpty, @@ -357,11 +316,11 @@ final class ContactDiscoveryV2Operation { /// If the lookup succeeds, we'll get back a PNI. If it doesn't succeed, the /// user with a particular e164 may not be registered, or they may have /// chosen to hide their phone number. - var pni: UUID + var pni: Pni /// If we provide the correct ACI-UAK pair, we'll also get back the ACI /// associated with the e164/PNI. - var aci: UUID? + var aci: Aci? } static func decodePniAciResult(_ data: Data) throws -> [DiscoveryResult] { @@ -383,17 +342,17 @@ final class ContactDiscoveryV2Operation { } remainingData = remainingData.dropFirst(rawE164Count) - guard let (pni, pniCount) = UUID.from(data: remainingData) else { + guard let (pniUuid, pniCount) = UUID.from(data: remainingData) else { throw ContactDiscoveryError.assertionError(description: "malformed e164/aci/pni triples") } remainingData = remainingData.dropFirst(pniCount) - guard let (aci, aciCount) = UUID.from(data: remainingData) else { + guard let (aciUuid, aciCount) = UUID.from(data: remainingData) else { throw ContactDiscoveryError.assertionError(description: "malformed e164/aci/pni triples") } remainingData = remainingData.dropFirst(aciCount) - guard pni != UUID.allZeros else { + guard pniUuid != UUID.allZeros else { return nil } guard let e164 = E164("+\(rawE164)") else { @@ -401,8 +360,8 @@ final class ContactDiscoveryV2Operation { } return DiscoveryResult( e164: e164, - pni: pni, - aci: aci == UUID.allZeros ? nil : aci + pni: Pni(fromUUID: pniUuid), + aci: aciUuid == UUID.allZeros ? nil : Aci(fromUUID: aciUuid) ) } } @@ -485,3 +444,33 @@ extension UUID { Self(data: Data(count: 16))! }() } + +// MARK: - Shims + +extension ContactDiscoveryV2Operation { + enum Shims { + typealias UDManager = _ContactDiscoveryV2Operation_UDManagerShim + } + + enum Wrappers { + typealias UDManager = _ContactDiscoveryV2Operation_UDManagerWrapper + } +} + +protocol _ContactDiscoveryV2Operation_UDManagerShim { + func fetchAllAciUakPairsWithSneakyTransaction() -> [AciObjC: SMKUDAccessKey] +} + +class _ContactDiscoveryV2Operation_UDManagerWrapper: _ContactDiscoveryV2Operation_UDManagerShim { + private let db: DB + private let udManager: OWSUDManager + + init(db: DB, udManager: OWSUDManager) { + self.db = db + self.udManager = udManager + } + + func fetchAllAciUakPairsWithSneakyTransaction() -> [AciObjC: SMKUDAccessKey] { + db.read { tx in udManager.fetchAllAciUakPairs(tx: SDSDB.shimOnlyBridge(tx)) } + } +} diff --git a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift index 3255f76417..168cf4b84d 100644 --- a/SignalServiceKit/src/Messages/UD/OWSUDManager.swift +++ b/SignalServiceKit/src/Messages/UD/OWSUDManager.swift @@ -159,6 +159,9 @@ public protocol OWSUDManager: AnyObject { @objc func storySendingAccess(for serviceId: UntypedServiceIdObjC, senderCertificates: SenderCertificates) -> OWSUDSendingAccess + @objc + func fetchAllAciUakPairs(tx: SDSAnyReadTransaction) -> [AciObjC: SMKUDAccessKey] + // MARK: Sender Certificate // We use completion handlers instead of a promise so that message sending @@ -224,6 +227,7 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { // // TODO: We might not want to use comprehensive caches here. private var phoneNumberAccessCache = [String: UnidentifiedAccessMode]() + // PNI TODO: Change this type to Aci or ServiceId. private var uuidAccessCache = [UUID: UnidentifiedAccessMode]() @objc @@ -428,21 +432,37 @@ public class OWSUDManagerImpl: NSObject, OWSUDManager { } } + public func fetchAllAciUakPairs(tx: SDSAnyReadTransaction) -> [AciObjC: SMKUDAccessKey] { + let acis = self.unfairLock.withLock { + self.uuidAccessCache.compactMap { (serviceId, mode) -> Aci? in + switch mode { + case .enabled, .unrestricted, .unknown: + return Aci(fromUUID: serviceId) + case .disabled: + return nil + } + } + } + var result = [AciObjC: SMKUDAccessKey]() + for aci in acis { + result[AciObjC(aci)] = udAccessKey(for: SignalServiceAddress(aci), tx: tx) + } + return result + } + // Returns the UD access key for a given recipient // if we have a valid profile key for them. @objc public func udAccessKey(forAddress address: SignalServiceAddress) -> SMKUDAccessKey? { - let profileKeyData = databaseStorage.read { transaction in - return self.profileManager.profileKeyData(for: address, - transaction: transaction) - } - guard let profileKey = profileKeyData else { - // Mark as "not a UD recipient". + return databaseStorage.read { tx in udAccessKey(for: address, tx: tx) } + } + + private func udAccessKey(for address: SignalServiceAddress, tx: SDSAnyReadTransaction) -> SMKUDAccessKey? { + guard let profileKey = profileManager.profileKeyData(for: address, transaction: tx) else { return nil } do { - let udAccessKey = try SMKUDAccessKey(profileKey: profileKey) - return udAccessKey + return try SMKUDAccessKey(profileKey: profileKey) } catch { Logger.error("Could not determine udAccessKey: \(error)") return nil diff --git a/SignalServiceKit/src/TestUtils/MockSSKEnvironment.swift b/SignalServiceKit/src/TestUtils/MockSSKEnvironment.swift index 9d34cc0f40..e5a381ccc6 100644 --- a/SignalServiceKit/src/TestUtils/MockSSKEnvironment.swift +++ b/SignalServiceKit/src/TestUtils/MockSSKEnvironment.swift @@ -143,6 +143,7 @@ public class MockSSKEnvironment: SSKEnvironment { recipientFetcher: dependenciesBridge.recipientFetcher, recipientMerger: dependenciesBridge.recipientMerger, tsAccountManager: tsAccountManager, + udManager: udManager, websocketFactory: webSocketFactory ) let messageSendLog = MessageSendLog(databaseStorage: databaseStorage, dateProvider: { Date() }) diff --git a/SignalServiceKit/src/Util/RemoteConfigManager.swift b/SignalServiceKit/src/Util/RemoteConfigManager.swift index d1bda1958b..43d9585186 100644 --- a/SignalServiceKit/src/Util/RemoteConfigManager.swift +++ b/SignalServiceKit/src/Util/RemoteConfigManager.swift @@ -256,6 +256,22 @@ public class RemoteConfig: BaseFlags { return .mirroring } + static var tryToReturnAcisWithoutUaks: Bool { + if !FeatureFlags.phoneNumberIdentifiers { + return true + } + if TSConstants.isUsingProductionService { + return true + } + if OWSIsDebugBuild() { + return true + } + if !isEnabled(.cdsDisableCompatibilityMode) { + return true + } + return false + } + // MARK: UInt values private static func getUIntValue( @@ -555,6 +571,7 @@ private struct Flags { case ringrtcNwPathMonitorTrialKillSwitch case stopMirroringToSVR2Override case exclusiveSVR2 + case cdsDisableCompatibilityMode } // Values defined in this array remain set once they are @@ -640,6 +657,7 @@ private extension FlagType { case "minNicknameLength": return "global.nicknames.min" case "maxNicknameLength": return "global.nicknames.max" case "safetyNumberAci": return "global.safetyNumberAci" + case "cdsDisableCompatibilityMode": return "cds.disableCompatibilityMode" default: return Flags.prefix + rawValue } } diff --git a/SignalServiceKit/tests/Contacts/Discovery/ContactDiscoveryV2OperationTest.swift b/SignalServiceKit/tests/Contacts/Discovery/ContactDiscoveryV2OperationTest.swift index 8866550dce..52f23aeb51 100644 --- a/SignalServiceKit/tests/Contacts/Discovery/ContactDiscoveryV2OperationTest.swift +++ b/SignalServiceKit/tests/Contacts/Discovery/ContactDiscoveryV2OperationTest.swift @@ -3,13 +3,19 @@ // SPDX-License-Identifier: AGPL-3.0-only // +import LibSignalClient import XCTest + @testable import SignalServiceKit final class ContactDiscoveryV2OperationTest: XCTestCase { // MARK: - Mocks + private class MockUDManager: ContactDiscoveryV2Operation.Shims.UDManager { + func fetchAllAciUakPairsWithSneakyTransaction() -> [AciObjC: SMKUDAccessKey] { return [:] } + } + private class MockContactDiscoveryV2PersistentState: ContactDiscoveryV2PersistentState { var token: Data? var prevE164s = Set() @@ -41,13 +47,14 @@ final class ContactDiscoveryV2OperationTest: XCTestCase { /// In .oneOffUserRequest mode, we should disregard tokens entirely. func testOneOffRequest() throws { - let aci = UUID() - let pni = UUID() + let aci = Aci.randomForTesting() + let pni = Pni.randomForTesting() let operation = ContactDiscoveryV2Operation( e164sToLookup: [try XCTUnwrap(E164("+16505550100"))], - compatibilityMode: .fetchAllACIs, + tryToReturnAcisWithoutUaks: true, persistentState: nil, + udManager: MockUDManager(), connectionFactory: connectionFactory ) @@ -68,7 +75,7 @@ final class ContactDiscoveryV2OperationTest: XCTestCase { XCTAssertTrue(request.tokenAck) var response = CDSI_ClientResponse() - response.e164PniAciTriples = newE164s! + pni.data + aci.data + response.e164PniAciTriples = newE164s! + pni.rawUUID.data + aci.rawUUID.data return .value([response]) } connectionFactory.setOnConnectAndPerformHandshake({ _ in @@ -94,8 +101,9 @@ final class ContactDiscoveryV2OperationTest: XCTestCase { func testNotDiscoverable() throws { let operation = ContactDiscoveryV2Operation( e164sToLookup: [try XCTUnwrap(E164("+16505550100"))], - compatibilityMode: .fetchAllACIs, + tryToReturnAcisWithoutUaks: true, persistentState: nil, + udManager: MockUDManager(), connectionFactory: connectionFactory ) @@ -132,8 +140,9 @@ final class ContactDiscoveryV2OperationTest: XCTestCase { func testRateLimitError() throws { let operation = ContactDiscoveryV2Operation( e164sToLookup: [try XCTUnwrap(E164("+16505550100"))], - compatibilityMode: .fetchAllACIs, + tryToReturnAcisWithoutUaks: true, persistentState: persistentState, + udManager: MockUDManager(), connectionFactory: connectionFactory ) @@ -177,8 +186,9 @@ final class ContactDiscoveryV2OperationTest: XCTestCase { func testInvalidTokenError() throws { let operation = ContactDiscoveryV2Operation( e164sToLookup: [try XCTUnwrap(E164("+16505550100"))], - compatibilityMode: .fetchAllACIs, + tryToReturnAcisWithoutUaks: true, persistentState: persistentState, + udManager: MockUDManager(), connectionFactory: connectionFactory )