diff --git a/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift b/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift index 1d4b7cfcb..e5efbc83b 100644 --- a/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift +++ b/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift @@ -138,7 +138,7 @@ class LegacyContactDiscoveryBatchOperation: OWSOperation { let cdsFeedbackOperation = CDSFeedbackOperation(legacyRegisteredRecipientIds: self.registeredRecipientIds) cdsFeedbackOperation.addDependency(newCDSBatchOperation) - CDSFeedbackOperation.operationQueue.addOperations([newCDSBatchOperation, cdsFeedbackOperation], waitUntilFinished: false) + CDSBatchOperation.operationQueue.addOperations([newCDSBatchOperation, cdsFeedbackOperation], waitUntilFinished: false) } // Called at most one time. @@ -190,16 +190,26 @@ class LegacyContactDiscoveryBatchOperation: OWSOperation { } +enum ContactDiscoveryError: Error { + case parseError(description: String) + case assertionError(description: String) + case attestationError(underlyingError: Error) + case clientError(underlyingError: Error) + case serverError(underlyingError: Error) +} + public class CDSBatchOperation: OWSOperation { - enum CDSBatchOperationError: Error { - case parseError(description: String) - case assertionError(description: String) - } - private let recipientIdsToLookup: [String] - var registeredRecipientIds: Set + private(set) var registeredRecipientIds: Set + + static let operationQueue: OperationQueue = { + let queue = OperationQueue() + queue.maxConcurrentOperationCount = 1 + + return queue + }() private var networkManager: TSNetworkManager { return TSNetworkManager.shared() @@ -235,7 +245,11 @@ class CDSBatchOperation: OWSOperation { contactDiscoveryService.performRemoteAttestation(success: { (remoteAttestation: RemoteAttestation) in self.makeContactDiscoveryRequest(remoteAttestation: remoteAttestation) }, - failure: self.reportError) + failure: self.attestationFailure) + } + + private func attestationFailure(error: Error) { + self.reportError(ContactDiscoveryError.attestationError(underlyingError: error)) } private func makeContactDiscoveryRequest(remoteAttestation: RemoteAttestation) { @@ -275,18 +289,33 @@ class CDSBatchOperation: OWSOperation { }, failure: { (task, error) in guard let response = task.response as? HTTPURLResponse else { - let responseError: NSError = OWSErrorMakeUnableToProcessServerResponseError() as NSError + let responseError = OWSErrorMakeUnableToProcessServerResponseError() as NSError responseError.isRetryable = true self.reportError(responseError) return } + // TODO CDS ratelimiting guard response.statusCode != 413 else { let rateLimitError = OWSErrorWithCodeDescription(OWSErrorCode.contactsUpdaterRateLimit, "Contacts Intersection Rate Limit") self.reportError(rateLimitError) return } + guard response.statusCode / 100 != 4 else { + let clientError: NSError = ContactDiscoveryError.clientError(underlyingError: error) as NSError + clientError.isRetryable = (error as NSError).isRetryable + self.reportError(clientError) + return + } + + guard response.statusCode / 100 != 5 else { + let serverError = ContactDiscoveryError.serverError(underlyingError: error) as NSError + serverError.isRetryable = (error as NSError).isRetryable + self.reportError(serverError) + return + } + self.reportError(error) }) } @@ -299,7 +328,7 @@ class CDSBatchOperation: OWSOperation { additionalAuthenticatedData: remoteAttestation.requestId, key: remoteAttestation.keys.clientKey) else { - throw CDSBatchOperationError.assertionError(description: "Encryption failure") + throw ContactDiscoveryError.assertionError(description: "Encryption failure") } return encryptionResult @@ -310,14 +339,14 @@ class CDSBatchOperation: OWSOperation { for recipientId in recipientIds { guard recipientId.prefix(1) == "+" else { - throw CDSBatchOperationError.assertionError(description: "unexpected id format") + throw ContactDiscoveryError.assertionError(description: "unexpected id format") } let numericPortionIndex = recipientId.index(after: recipientId.startIndex) let numericPortion = recipientId.suffix(from: numericPortionIndex) guard let numericIdentifier = UInt64(numericPortion), numericIdentifier > 99 else { - throw CDSBatchOperationError.assertionError(description: "unexpectedly short identifier") + throw ContactDiscoveryError.assertionError(description: "unexpectedly short identifier") } var bigEndian: UInt64 = CFSwapInt64HostToBig(numericIdentifier) @@ -331,7 +360,7 @@ class CDSBatchOperation: OWSOperation { func handle(response: Any?, remoteAttestation: RemoteAttestation) throws -> Set { let isIncludedData: Data = try parseAndDecrypt(response: response, remoteAttestation: remoteAttestation) guard let isIncluded: [Bool] = type(of: self).boolArray(data: isIncludedData) else { - throw CDSBatchOperationError.assertionError(description: "isIncluded was unexpectedly nil") + throw ContactDiscoveryError.assertionError(description: "isIncluded was unexpectedly nil") } return try match(recipientIds: self.recipientIdsToLookup, isIncluded: isIncluded) @@ -349,7 +378,7 @@ class CDSBatchOperation: OWSOperation { func match(recipientIds: [String], isIncluded: [Bool]) throws -> Set { guard recipientIds.count == isIncluded.count else { - throw CDSBatchOperationError.assertionError(description: "length mismatch for isIncluded/recipientIds") + throw ContactDiscoveryError.assertionError(description: "length mismatch for isIncluded/recipientIds") } let includedRecipientIds: [String] = (0.. Data { guard let responseDict = response as? [String: AnyObject] else { - throw CDSBatchOperationError.parseError(description: "missing response dict") + throw ContactDiscoveryError.parseError(description: "missing response dict") } let cipherText = try responseDict.expectBase64EncodedData(key: "data") @@ -375,7 +404,7 @@ class CDSBatchOperation: OWSOperation { authTag: authTag, key: remoteAttestation.keys.serverKey) else { - throw CDSBatchOperationError.parseError(description: "decryption failed") + throw ContactDiscoveryError.parseError(description: "decryption failed") } return plainText @@ -384,15 +413,21 @@ class CDSBatchOperation: OWSOperation { class CDSFeedbackOperation: OWSOperation { - static let operationQueue: OperationQueue = { - let queue = OperationQueue() - queue.maxConcurrentOperationCount = 1 - - return queue - }() + enum FeedbackResult: String { + case ok + case mismatch + case serverError = "server-error" + case clientError = "client-error" + case attestationError = "attestation-error" + case unexpectedError = "unexpected-error" + } private let legacyRegisteredRecipientIds: Set + var networkManager: TSNetworkManager { + return TSNetworkManager.shared() + } + // MARK: Initializers required init(legacyRegisteredRecipientIds: Set) { @@ -405,33 +440,57 @@ class CDSFeedbackOperation: OWSOperation { // MARK: OWSOperation Overrides + override func checkForPreconditionError() -> Error? { + // override super with no-op + // In this rare case, we want to proceed even though our dependency might have an + // error so we can report the details of that error to the feedback service. + return nil + } + // Called every retry, this is where the bulk of the operation's work should go. override func run() { + + guard !isCancelled else { + Logger.info("\(logTag) in \(#function) no work to do, since we were canceled") + self.reportCancelled() + return + } + guard let cdsOperation = dependencies.first as? CDSBatchOperation else { let error = OWSErrorMakeAssertionError("\(self.logTag) in \(#function) cdsOperation was unexpectedly nil") self.reportError(error) return } - let cdsRegisteredRecipientIds = cdsOperation.registeredRecipientIds + if let error = cdsOperation.failingError { + switch error { + case ContactDiscoveryError.serverError: + self.makeRequest(result: .serverError) + case ContactDiscoveryError.clientError: + self.makeRequest(result: .clientError) + case ContactDiscoveryError.attestationError: + self.makeRequest(result: .attestationError) + default: + self.makeRequest(result: .unexpectedError) + } - if cdsRegisteredRecipientIds == legacyRegisteredRecipientIds { - Logger.debug("\(logTag) in \(#function) TODO: PUT /v1/directory/feedback/ok") - } else { - Logger.debug("\(logTag) in \(#function) TODO: PUT /v1/directory/feedback/mismatch") + return } - self.reportSuccess() + if cdsOperation.registeredRecipientIds == legacyRegisteredRecipientIds { + self.makeRequest(result: .ok) + return + } else { + self.makeRequest(result: .mismatch) + return + } } - override func didFail(error: Error) { - // dependency failed. - // Depending on error, PUT one of: - // /v1/directory/feedback/server-error: - // /v1/directory/feedback/client-error: - // /v1/directory/feedback/attestation-error: - // /v1/directory/feedback/unexpected-error: - Logger.debug("\(logTag) in \(#function) TODO: PUT /v1/directory/feedback/*-error") + func makeRequest(result: FeedbackResult) { + let request = OWSRequestFactory.cdsFeedbackRequest(result: result.rawValue) + self.networkManager.makeRequest(request, + success: { _, _ in self.reportSuccess() }, + failure: { _, error in self.reportError(error) }) } } diff --git a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h index 4dd117da6..29a8a489a 100644 --- a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h +++ b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h @@ -86,6 +86,7 @@ typedef NS_ENUM(NSUInteger, TSVerificationTransport) { TSVerificationTransportVo cookies:(NSArray *)cookies; + (TSRequest *)remoteAttestationAuthRequest; ++ (TSRequest *)cdsFeedbackRequestWithResult:(NSString *)result NS_SWIFT_NAME(cdsFeedbackRequest(result:)); @end diff --git a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m index 6ac5d57e1..8beb06972 100644 --- a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m +++ b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m @@ -340,6 +340,12 @@ NS_ASSUME_NONNULL_BEGIN return [TSRequest requestWithUrl:[NSURL URLWithString:path] method:@"GET" parameters:@{}]; } ++ (TSRequest *)cdsFeedbackRequestWithResult:(NSString *)result +{ + NSString *path = [NSString stringWithFormat:@"/v1/directory/feedback/%@", result]; + return [TSRequest requestWithUrl:[NSURL URLWithString:path] method:@"PUT" parameters:@{}]; +} + @end NS_ASSUME_NONNULL_END