From bec6cdac0522e2ca0eda28e3a6032e8878fd5d05 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Tue, 21 Nov 2023 10:59:10 +0200 Subject: [PATCH] Secure backup tweaks (#2122) * Show the recovery section if key backup is anything **but** disabled * Don't show an action on secure backup if its current state is unknown * Do not show a badge on the home screen avatar while the recovery state is unknown * Do not assume key backup is disabled when its state is unknown. If the state is unknown, check if a backup exists remotely. * Dedupe remote backup state requests and handle task cancellations --- .../HomeScreen/HomeScreenViewModel.swift | 2 +- .../View/SecureBackupScreen.swift | 6 ++- .../SecureBackup/SecureBackupController.swift | 40 ++++++++++++++++++- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift index d4be6994b..81834dcd5 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift @@ -85,7 +85,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol .sink { [weak self] recoveryKeyState in guard let self, appSettings.chatBackupEnabled else { return } - let requiresSecureBackupSetup = recoveryKeyState == .unknown || recoveryKeyState == .disabled || recoveryKeyState == .incomplete + let requiresSecureBackupSetup = recoveryKeyState == .disabled || recoveryKeyState == .incomplete state.showUserMenuBadge = requiresSecureBackupSetup state.showSettingsMenuOptionBadge = requiresSecureBackupSetup diff --git a/ElementX/Sources/Screens/SecureBackup/SecureBackupScreen/View/SecureBackupScreen.swift b/ElementX/Sources/Screens/SecureBackup/SecureBackupScreen/View/SecureBackupScreen.swift index 990478c7b..0f19bf279 100644 --- a/ElementX/Sources/Screens/SecureBackup/SecureBackupScreen/View/SecureBackupScreen.swift +++ b/ElementX/Sources/Screens/SecureBackup/SecureBackupScreen/View/SecureBackupScreen.swift @@ -23,7 +23,7 @@ struct SecureBackupScreen: View { var body: some View { Form { - if context.viewState.keyBackupState != .enabled { + if context.viewState.keyBackupState == .disabled { keyBackupSection } else { keyBackupSection @@ -77,10 +77,12 @@ struct SecureBackupScreen: View { ListRow(label: .plain(title: L10n.screenChatBackupKeyBackupActionDisable, role: .destructive), kind: .navigationLink { context.send(viewAction: .keyBackup) }) - case .disabled, .enabling, .unknown: + case .disabled, .enabling: ListRow(label: .plain(title: L10n.screenChatBackupKeyBackupActionEnable), kind: .navigationLink { context.send(viewAction: .keyBackup) }) + case .unknown: + EmptyView() } } diff --git a/ElementX/Sources/Services/SecureBackup/SecureBackupController.swift b/ElementX/Sources/Services/SecureBackup/SecureBackupController.swift index f5d7f5d7e..95cbde9c7 100644 --- a/ElementX/Sources/Services/SecureBackup/SecureBackupController.swift +++ b/ElementX/Sources/Services/SecureBackup/SecureBackupController.swift @@ -27,6 +27,9 @@ class SecureBackupController: SecureBackupControllerProtocol { private var backupStateListenerTaskHandle: TaskHandle? private var recoveryStateListenerTaskHandle: TaskHandle? + /// Used to dedupe remote backup state requests + @CancellableTask private var remoteBackupStateTask: Task? + var recoveryKeyState: CurrentValuePublisher { recoveryKeyStateSubject.asCurrentValuePublisher() } @@ -39,11 +42,18 @@ class SecureBackupController: SecureBackupControllerProtocol { self.encryption = encryption backupStateListenerTaskHandle = encryption.backupStateListener(listener: SecureBackupControllerBackupStateListener { [weak self] state in - self?.keyBackupStateSubject.send(state.keyBackupState) + guard let self else { return } + + keyBackupStateSubject.send(state.keyBackupState) + + if case .unknown = state { + updateBackupStateFromRemote() + } }) recoveryStateListenerTaskHandle = encryption.recoveryStateListener(listener: SecureBackupRecoveryStateListener { [weak self] state in guard let self else { return } + switch state { case .unknown: recoveryKeyStateSubject.send(.unknown) @@ -55,6 +65,8 @@ class SecureBackupController: SecureBackupControllerProtocol { recoveryKeyStateSubject.send(.incomplete) } }) + + updateBackupStateFromRemote() } func enable() async -> Result { @@ -140,6 +152,30 @@ class SecureBackupController: SecureBackupControllerProtocol { return .failure(.failedUploadingForBackup) } } + + // MARK: - Private + + private func updateBackupStateFromRemote(retry: Bool = true) { + remoteBackupStateTask = Task { + do { + let backupExists = try await self.encryption.backupExistsOnServer() + + if Task.isCancelled { + return + } + + if !backupExists { + keyBackupStateSubject.send(.disabled) + } + } catch { + MXLog.error("Failed retrieving remote backup state with error: \(error)") + + if retry { + updateBackupStateFromRemote(retry: false) + } + } + } + } } private final class SecureBackupControllerBackupStateListener: BackupStateListener { @@ -182,7 +218,7 @@ extension BackupState { var keyBackupState: SecureBackupKeyBackupState { switch self { case .unknown: - return .disabled + return .unknown case .creating: return .enabling case .enabling: