From 94a925cac3c7bf1549a5a7b53d8d41aa667a189d Mon Sep 17 00:00:00 2001 From: Jon Petersson Date: Wed, 29 Nov 2023 16:17:38 +0100 Subject: [PATCH] Refactor account deletion in TunnelManager --- ios/MullvadVPN.xcodeproj/project.pbxproj | 6 -- .../DeleteAccountOperation.swift | 61 ---------------- .../TunnelManager/SetAccountOperation.swift | 69 +++++++++++++++++++ .../TunnelManager/TunnelInteractor.swift | 1 + .../TunnelManager/TunnelManager.swift | 56 ++++----------- .../AccountDeletionInteractor.swift | 4 +- .../AccountDeletionViewController.swift | 4 +- 7 files changed, 88 insertions(+), 113 deletions(-) delete mode 100644 ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj index 2bec1aba1358..d0de6a0bff3e 100644 --- a/ios/MullvadVPN.xcodeproj/project.pbxproj +++ b/ios/MullvadVPN.xcodeproj/project.pbxproj @@ -570,7 +570,6 @@ A9A5FA0F2ACB05160083449F /* StoreSubscription.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5846227026E229F20035F7C2 /* StoreSubscription.swift */; }; A9A5FA102ACB05160083449F /* PacketTunnelTransport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 063687B928EB234F00BE7161 /* PacketTunnelTransport.swift */; }; A9A5FA112ACB05160083449F /* TransportMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0697D6E628F01513007A9E99 /* TransportMonitor.swift */; }; - A9A5FA122ACB05160083449F /* DeleteAccountOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */; }; A9A5FA132ACB05160083449F /* LoadTunnelConfigurationOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 588527B1276B3F0700BAA373 /* LoadTunnelConfigurationOperation.swift */; }; A9A5FA142ACB05160083449F /* MapConnectionStatusOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58F2E147276A307400A79513 /* MapConnectionStatusOperation.swift */; }; A9A5FA152ACB05160083449F /* RedeemVoucherOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = F07BF2612A26279100042943 /* RedeemVoucherOperation.swift */; }; @@ -666,7 +665,6 @@ F0B0E6972AFE6E7E001DC66B /* XCTest+Async.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0B0E6962AFE6E7E001DC66B /* XCTest+Async.swift */; }; F0C2AEFD2A0BB5CC00986207 /* NotificationProviderIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C2AEFC2A0BB5CC00986207 /* NotificationProviderIdentifier.swift */; }; F0C6A8432AB08E54000777A8 /* RedeemVoucherViewConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6A8422AB08E54000777A8 /* RedeemVoucherViewConfiguration.swift */; }; - F0C6FA812A66E23300F521F0 /* DeleteAccountOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */; }; F0C6FA852A6A733700F521F0 /* InAppPurchaseInteractor.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6FA842A6A733700F521F0 /* InAppPurchaseInteractor.swift */; }; F0D8825B2B04F53600D3EF9A /* OutgoingConnectionData.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0D8825A2B04F53600D3EF9A /* OutgoingConnectionData.swift */; }; F0D8825C2B04F70E00D3EF9A /* OutgoingConnectionData.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0D8825A2B04F53600D3EF9A /* OutgoingConnectionData.swift */; }; @@ -1666,7 +1664,6 @@ F0B0E6962AFE6E7E001DC66B /* XCTest+Async.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "XCTest+Async.swift"; sourceTree = ""; }; F0C2AEFC2A0BB5CC00986207 /* NotificationProviderIdentifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationProviderIdentifier.swift; sourceTree = ""; }; F0C6A8422AB08E54000777A8 /* RedeemVoucherViewConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RedeemVoucherViewConfiguration.swift; sourceTree = ""; }; - F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeleteAccountOperation.swift; sourceTree = ""; }; F0C6FA842A6A733700F521F0 /* InAppPurchaseInteractor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InAppPurchaseInteractor.swift; sourceTree = ""; }; F0D8825A2B04F53600D3EF9A /* OutgoingConnectionData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OutgoingConnectionData.swift; sourceTree = ""; }; F0DA87462A9CB9A2006044F1 /* AccountExpiryRow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountExpiryRow.swift; sourceTree = ""; }; @@ -1995,7 +1992,6 @@ 5823FA5726CE4A4100283BF8 /* TunnelManager */ = { isa = PBXGroup; children = ( - F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */, 588527B1276B3F0700BAA373 /* LoadTunnelConfigurationOperation.swift */, 58F2E147276A307400A79513 /* MapConnectionStatusOperation.swift */, F07BF2612A26279100042943 /* RedeemVoucherOperation.swift */, @@ -4193,7 +4189,6 @@ A9A5FA0F2ACB05160083449F /* StoreSubscription.swift in Sources */, A9A5FA102ACB05160083449F /* PacketTunnelTransport.swift in Sources */, A9A5FA112ACB05160083449F /* TransportMonitor.swift in Sources */, - A9A5FA122ACB05160083449F /* DeleteAccountOperation.swift in Sources */, A9B6AC1A2ADE8FBB00F7802A /* InMemorySettingsStore.swift in Sources */, A9A5FA132ACB05160083449F /* LoadTunnelConfigurationOperation.swift in Sources */, A9A5FA142ACB05160083449F /* MapConnectionStatusOperation.swift in Sources */, @@ -4491,7 +4486,6 @@ 7AF9BE8E2A331C7B00DBFEDB /* RelayFilterViewModel.swift in Sources */, 58F3C0A4249CB069003E76BE /* HeaderBarView.swift in Sources */, 5864AF0829C78849005B0CD9 /* CellFactoryProtocol.swift in Sources */, - F0C6FA812A66E23300F521F0 /* DeleteAccountOperation.swift in Sources */, F07CFF2029F2720E008C0343 /* RegisteredDeviceInAppNotificationProvider.swift in Sources */, 587A01FC23F1F0BE00B68763 /* SimulatorTunnelProviderHost.swift in Sources */, 7A6F2FA72AFBB9AE006D0856 /* AccountExpiry.swift in Sources */, diff --git a/ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift b/ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift deleted file mode 100644 index 8731ca2a5cf1..000000000000 --- a/ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift +++ /dev/null @@ -1,61 +0,0 @@ -// -// DeleteAccountOperation.swift -// MullvadVPN -// -// Created by Mojgan on 2023-07-18. -// Copyright © 2023 Mullvad VPN AB. All rights reserved. -// - -import Foundation -import MullvadLogging -import MullvadREST -import MullvadTypes -import Operations - -class DeleteAccountOperation: ResultOperation { - private let logger = Logger(label: "\(DeleteAccountOperation.self)") - - private let accountNumber: String - private let accountsProxy: RESTAccountHandling - private let accessTokenManager: RESTAccessTokenManagement - private var task: Cancellable? - - init( - dispatchQueue: DispatchQueue, - accountsProxy: RESTAccountHandling, - accessTokenManager: RESTAccessTokenManagement, - accountNumber: String - ) { - self.accountNumber = accountNumber - self.accountsProxy = accountsProxy - self.accessTokenManager = accessTokenManager - super.init(dispatchQueue: dispatchQueue) - } - - override func main() { - task = accountsProxy.deleteAccount( - accountNumber: accountNumber, - retryStrategy: .default, - completion: { [weak self] result in - self?.dispatchQueue.async { - switch result { - case .success: - self?.accessTokenManager.invalidateAllTokens() - self?.finish(result: .success(())) - case let .failure(error): - self?.logger.error( - error: error, - message: "Failed to delete account." - ) - self?.finish(result: .failure(error)) - } - } - } - ) - } - - override func operationDidCancel() { - task?.cancel() - task = nil - } -} diff --git a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift index 1f8307d2b7e3..4992a76e5145 100644 --- a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift +++ b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift @@ -24,11 +24,15 @@ enum SetAccountAction { /// Unset account. case unset + /// Delete account. + case delete(String) + var taskName: String { switch self { case .new: "Set new account" case .existing: "Set existing account" case .unset: "Unset account" + case .delete: "Delete account" } } } @@ -78,6 +82,11 @@ class SetAccountOperation: ResultOperation { case .unset: finish(result: .success(nil)) + + case let .delete(accountNumber): + startDeleteAccountFlow(accountNumber: accountNumber) { [self] result in + finish(result: result.map { .none }) + } } } } @@ -141,6 +150,42 @@ class SetAccountOperation: ResultOperation { } } + /** + Begin delete flow of an existing account by performing the following steps: + + 1. Delete existing account with the API. + 2. Reset tunnel settings to default and remove last used account. + */ + private func startDeleteAccountFlow( + accountNumber: String, + completion: @escaping (Result) -> Void + ) { + deleteAccount(accountNumber: accountNumber) { [self] result in + interactor.setSettings(LatestTunnelSettings(), persist: true) + + if result.isSuccess { + interactor.removeLastUsedAccount() + } + + completion(result) + } + } + +// operation.completionHandler = { [weak self] result in +// switch result { +// case .success: +// self?.unsetTunnelConfiguration { +// self?.operationQueue.cancelAllOperations() +// self?.wipeAllUserData() +// self?.setDeviceState(.loggedOut, persist: true) +// DispatchQueue.main.async { +// completion?(nil) +// } +// } +// case let .failure(error): +// completion?(error) +// } + /** Continue login flow after receiving account data as a part of creating new or retrieving existing account from the API by performing the following steps: @@ -264,6 +309,28 @@ class SetAccountOperation: ResultOperation { tasks.append(task) } + /// Delete account. + private func deleteAccount(accountNumber: String, completion: @escaping (Result) -> Void) { + logger.debug("Delete account...") + + let task = accountsProxy.deleteAccount( + accountNumber: accountNumber, + retryStrategy: .default + ) { [self] result in + dispatchQueue.async { [self] in + let result = result.inspectError { error in + guard !error.isOperationCancellationError else { return } + + logger.error(error: error, message: "Failed to delete account.") + } + + completion(result) + } + } + + tasks.append(task) + } + /// Delete device from API. private func deleteDevice(accountNumber: String, deviceIdentifier: String, completion: @escaping (Error?) -> Void) { logger.debug("Delete current device...") @@ -398,3 +465,5 @@ class SetAccountOperation: ResultOperation { var device: Device } } + +// swiftlint:disable:this file_length diff --git a/ios/MullvadVPN/TunnelManager/TunnelInteractor.swift b/ios/MullvadVPN/TunnelManager/TunnelInteractor.swift index 15f118f0352e..3ec3a9791bf8 100644 --- a/ios/MullvadVPN/TunnelManager/TunnelInteractor.swift +++ b/ios/MullvadVPN/TunnelManager/TunnelInteractor.swift @@ -33,6 +33,7 @@ protocol TunnelInteractor { func setConfigurationLoaded() func setSettings(_ settings: LatestTunnelSettings, persist: Bool) func setDeviceState(_ deviceState: DeviceState, persist: Bool) + func removeLastUsedAccount() func handleRestError(_ error: Error) func startTunnel() diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift index 77e1eb24cdfc..35982146acf8 100644 --- a/ios/MullvadVPN/TunnelManager/TunnelManager.swift +++ b/ios/MullvadVPN/TunnelManager/TunnelManager.swift @@ -364,10 +364,13 @@ final class TunnelManager: StorePaymentObserver { MutuallyExclusive(category: OperationCategory.settingsUpdate.category) ) - // Unsetting the account (ie. logging out) should cancel all other currently ongoing - // activity. - if case .unset = action { + // Unsetting (ie. logging out) or deleting the account should cancel all other + // currently ongoing activity. + switch action { + case .unset, .delete: operationQueue.cancelAllOperations() + default: + break } operationQueue.addOperation(operation) @@ -437,43 +440,10 @@ final class TunnelManager: StorePaymentObserver { func deleteAccount( accountNumber: String, completion: ((Error?) -> Void)? = nil - ) -> Cancellable { - let operation = DeleteAccountOperation( - dispatchQueue: internalQueue, - accountsProxy: accountsProxy, - accessTokenManager: accessTokenManager, - accountNumber: accountNumber - ) - - operation.completionQueue = .main - operation.completionHandler = { [weak self] result in - switch result { - case .success: - self?.unsetTunnelConfiguration { - self?.operationQueue.cancelAllOperations() - self?.wipeAllUserData() - self?.setDeviceState(.loggedOut, persist: true) - DispatchQueue.main.async { - completion?(nil) - } - } - case let .failure(error): - completion?(error) - } + ) { + setAccount(action: .delete(accountNumber)) { [weak self] result in + completion?(result.error) } - - operation.addObserver( - BackgroundObserver( - application: application, - name: "Delete account", - cancelUponExpiration: true - ) - ) - - operation.addCondition(MutuallyExclusive(category: OperationCategory.deviceStateUpdate.category)) - - operationQueue.addOperation(operation) - return operation } func updateDeviceData(_ completionHandler: ((Error?) -> Void)? = nil) { @@ -1091,7 +1061,7 @@ final class TunnelManager: StorePaymentObserver { isRunningPeriodicPrivateKeyRotation = false } - private func wipeAllUserData() { + fileprivate func removeLastUsedAccount() { do { try SettingsManager.setLastUsedAccount(nil) } catch { @@ -1120,7 +1090,7 @@ final class TunnelManager: StorePaymentObserver { unsetTunnelConfiguration { self.setDeviceState(.revoked, persist: true) self.operationQueue.cancelAllOperations() - self.wipeAllUserData() + self.removeLastUsedAccount() } default: break @@ -1306,6 +1276,10 @@ private struct TunnelInteractorProxy: TunnelInteractor { tunnelManager.setDeviceState(deviceState, persist: persist) } + func removeLastUsedAccount() { + tunnelManager.removeLastUsedAccount() + } + func startTunnel() { tunnelManager.startTunnel() } diff --git a/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionInteractor.swift b/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionInteractor.swift index 50bf101f4fad..1562c013b835 100644 --- a/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionInteractor.swift +++ b/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionInteractor.swift @@ -48,7 +48,7 @@ class AccountDeletionInteractor { } } - func delete(accountNumber: String, completionHandler: @escaping (Error?) -> Void) -> Cancellable { - return tunnelManager.deleteAccount(accountNumber: accountNumber, completion: completionHandler) + func delete(accountNumber: String, completionHandler: @escaping (Error?) -> Void) { + tunnelManager.deleteAccount(accountNumber: accountNumber, completion: completionHandler) } } diff --git a/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionViewController.swift b/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionViewController.swift index 8fce5224aa53..cea4095b75c1 100644 --- a/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionViewController.swift +++ b/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionViewController.swift @@ -15,7 +15,6 @@ protocol AccountDeletionViewControllerDelegate: AnyObject { } class AccountDeletionViewController: UIViewController { - private var task: Cancellable? private lazy var contentView: AccountDeletionContentView = { let view = AccountDeletionContentView() view.delegate = self @@ -63,7 +62,7 @@ class AccountDeletionViewController: UIViewController { private func submit(accountNumber: String) { contentView.state = .loading - task = interactor.delete(accountNumber: accountNumber) { [weak self] error in + interactor.delete(accountNumber: accountNumber) { [weak self] error in guard let self else { return } guard let error else { self.contentView.state = .initial @@ -78,7 +77,6 @@ class AccountDeletionViewController: UIViewController { extension AccountDeletionViewController: AccountDeletionContentViewDelegate { func didTapCancelButton(contentView: AccountDeletionContentView, button: AppButton) { contentView.isEditing = false - task?.cancel() delegate?.deleteAccountDidCancel(controller: self) }