More secure backup tweaks (#2212)

* Make sure the indicator for disabling backups is dismissed together with the screen

* Get rid of the .disabled key backup state as it has no correspondant on the rust side and is confusing

* Remove superflous BackupState extensions, add more logs
This commit is contained in:
Stefan Ceriu 2023-12-07 13:19:58 +02:00 committed by GitHub
parent e8b446c909
commit 495ee8ab38
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 64 additions and 34 deletions

View File

@ -21,6 +21,7 @@ typealias SecureBackupKeyBackupScreenViewModelType = StateStoreViewModel<SecureB
class SecureBackupKeyBackupScreenViewModel: SecureBackupKeyBackupScreenViewModelType, SecureBackupKeyBackupScreenViewModelProtocol { class SecureBackupKeyBackupScreenViewModel: SecureBackupKeyBackupScreenViewModelType, SecureBackupKeyBackupScreenViewModelProtocol {
private let secureBackupController: SecureBackupControllerProtocol private let secureBackupController: SecureBackupControllerProtocol
private let userIndicatorController: UserIndicatorControllerProtocol?
private var actionsSubject: PassthroughSubject<SecureBackupKeyBackupScreenViewModelAction, Never> = .init() private var actionsSubject: PassthroughSubject<SecureBackupKeyBackupScreenViewModelAction, Never> = .init()
var actions: AnyPublisher<SecureBackupKeyBackupScreenViewModelAction, Never> { var actions: AnyPublisher<SecureBackupKeyBackupScreenViewModelAction, Never> {
@ -29,18 +30,18 @@ class SecureBackupKeyBackupScreenViewModel: SecureBackupKeyBackupScreenViewModel
init(secureBackupController: SecureBackupControllerProtocol, userIndicatorController: UserIndicatorControllerProtocol?) { init(secureBackupController: SecureBackupControllerProtocol, userIndicatorController: UserIndicatorControllerProtocol?) {
self.secureBackupController = secureBackupController self.secureBackupController = secureBackupController
self.userIndicatorController = userIndicatorController
super.init(initialViewState: .init(mode: secureBackupController.keyBackupState.value.viewMode)) super.init(initialViewState: .init(mode: secureBackupController.keyBackupState.value.viewMode))
secureBackupController.keyBackupState secureBackupController.keyBackupState
.receive(on: DispatchQueue.main) .receive(on: DispatchQueue.main)
.sink { [weak userIndicatorController] state in .sink { [weak self] state in
let loadingIndicatorIdentifier = "SecureBackupKeyBackupScreenLoading"
switch state { switch state {
case .disabling, .enabling, .unknown: case .disabling, .enabling, .unknown:
userIndicatorController?.submitIndicator(.init(id: loadingIndicatorIdentifier, type: .modal, title: L10n.commonLoading, persistent: true)) self?.showLoadingIndicator()
default: default:
userIndicatorController?.retractIndicatorWithId(loadingIndicatorIdentifier) self?.dismissLoadingIndicator()
} }
} }
.store(in: &cancellables) .store(in: &cancellables)
@ -80,8 +81,20 @@ class SecureBackupKeyBackupScreenViewModel: SecureBackupKeyBackupScreenViewModel
MXLog.error("Failed disabling key backup with error: \(error)") MXLog.error("Failed disabling key backup with error: \(error)")
state.bindings.alertInfo = .init(id: .init()) state.bindings.alertInfo = .init(id: .init())
} }
dismissLoadingIndicator()
} }
} }
private static let loadingIndicatorIdentifier = "SecureBackupKeyBackupScreenLoading"
private func showLoadingIndicator() {
userIndicatorController?.submitIndicator(.init(id: Self.loadingIndicatorIdentifier, type: .modal, title: L10n.commonLoading, persistent: true))
}
private func dismissLoadingIndicator() {
userIndicatorController?.retractIndicatorWithId(Self.loadingIndicatorIdentifier)
}
} }
extension SecureBackupKeyBackupState { extension SecureBackupKeyBackupState {

View File

@ -55,7 +55,7 @@ class SecureBackupScreenViewModel: SecureBackupScreenViewModelType, SecureBackup
actionsSubject.send(.recoveryKey) actionsSubject.send(.recoveryKey)
case .keyBackup: case .keyBackup:
switch secureBackupController.keyBackupState.value { switch secureBackupController.keyBackupState.value {
case .disabled, .unknown: case .unknown:
enableBackup() enableBackup()
case .enabled: case .enabled:
actionsSubject.send(.keyBackup) actionsSubject.send(.keyBackup)

View File

@ -23,7 +23,7 @@ struct SecureBackupScreen: View {
var body: some View { var body: some View {
Form { Form {
if context.viewState.keyBackupState == .disabled { if context.viewState.keyBackupState == .unknown {
keyBackupSection keyBackupSection
} else { } else {
keyBackupSection keyBackupSection
@ -77,12 +77,10 @@ struct SecureBackupScreen: View {
ListRow(label: .plain(title: L10n.screenChatBackupKeyBackupActionDisable, role: .destructive), kind: .navigationLink { ListRow(label: .plain(title: L10n.screenChatBackupKeyBackupActionDisable, role: .destructive), kind: .navigationLink {
context.send(viewAction: .keyBackup) context.send(viewAction: .keyBackup)
}) })
case .disabled, .enabling: case .unknown, .enabling:
ListRow(label: .plain(title: L10n.screenChatBackupKeyBackupActionEnable), kind: .navigationLink { ListRow(label: .plain(title: L10n.screenChatBackupKeyBackupActionEnable), kind: .navigationLink {
context.send(viewAction: .keyBackup) context.send(viewAction: .keyBackup)
}) })
case .unknown:
EmptyView()
} }
} }
@ -128,7 +126,7 @@ struct SecureBackupScreen: View {
struct SecureBackupScreen_Previews: PreviewProvider, TestablePreview { struct SecureBackupScreen_Previews: PreviewProvider, TestablePreview {
static let bothSetupViewModel = viewModel(keyBackupState: .enabled, recoveryKeyState: .enabled) static let bothSetupViewModel = viewModel(keyBackupState: .enabled, recoveryKeyState: .enabled)
static let onlyKeyBackupSetUpViewModel = viewModel(keyBackupState: .enabled, recoveryKeyState: .disabled) static let onlyKeyBackupSetUpViewModel = viewModel(keyBackupState: .enabled, recoveryKeyState: .disabled)
static let keyBackupDisabledViewModel = viewModel(keyBackupState: .disabled, recoveryKeyState: .disabled) static let keyBackupDisabledViewModel = viewModel(keyBackupState: .unknown, recoveryKeyState: .disabled)
static let recoveryIncompleteViewModel = viewModel(keyBackupState: .enabled, recoveryKeyState: .incomplete) static let recoveryIncompleteViewModel = viewModel(keyBackupState: .enabled, recoveryKeyState: .incomplete)
static var previews: some View { static var previews: some View {

View File

@ -44,7 +44,24 @@ class SecureBackupController: SecureBackupControllerProtocol {
backupStateListenerTaskHandle = encryption.backupStateListener(listener: SecureBackupControllerBackupStateListener { [weak self] state in backupStateListenerTaskHandle = encryption.backupStateListener(listener: SecureBackupControllerBackupStateListener { [weak self] state in
guard let self else { return } guard let self else { return }
keyBackupStateSubject.send(state.keyBackupState) switch state {
case .unknown:
keyBackupStateSubject.send(.unknown)
case .creating:
keyBackupStateSubject.send(.enabling)
case .enabling:
keyBackupStateSubject.send(.enabling)
case .resuming:
keyBackupStateSubject.send(.enabled)
case .enabled:
keyBackupStateSubject.send(.enabled)
case .downloading:
keyBackupStateSubject.send(.enabled)
case .disabling:
keyBackupStateSubject.send(.disabling)
}
MXLog.info("Key backup state changed to: \(state), setting local state to \(keyBackupStateSubject.value)")
if case .unknown = state { if case .unknown = state {
updateBackupStateFromRemote() updateBackupStateFromRemote()
@ -64,15 +81,21 @@ class SecureBackupController: SecureBackupControllerProtocol {
case .incomplete: case .incomplete:
recoveryKeyStateSubject.send(.incomplete) recoveryKeyStateSubject.send(.incomplete)
} }
MXLog.info("Recovery state changed to: \(state), setting local state to \(recoveryKeyStateSubject.value)")
}) })
updateBackupStateFromRemote() updateBackupStateFromRemote()
} }
func enable() async -> Result<Void, SecureBackupControllerError> { func enable() async -> Result<Void, SecureBackupControllerError> {
MXLog.info("Enabling secure backup")
do { do {
try await encryption.enableBackups() try await encryption.enableBackups()
} catch { } catch {
MXLog.error("Failed enabling secure backup with error: \(error)")
return .failure(.failedEnablingBackup) return .failure(.failedEnablingBackup)
} }
@ -80,9 +103,12 @@ class SecureBackupController: SecureBackupControllerProtocol {
} }
func disable() async -> Result<Void, SecureBackupControllerError> { func disable() async -> Result<Void, SecureBackupControllerError> {
MXLog.info("Disabling secure backup")
do { do {
try await encryption.disableRecovery() try await encryption.disableRecovery()
} catch { } catch {
MXLog.error("Failed disabling secure backup with error: \(error)")
return .failure(.failedDisablingBackup) return .failure(.failedDisablingBackup)
} }
@ -92,10 +118,14 @@ class SecureBackupController: SecureBackupControllerProtocol {
func generateRecoveryKey() async -> Result<String, SecureBackupControllerError> { func generateRecoveryKey() async -> Result<String, SecureBackupControllerError> {
do { do {
guard recoveryKeyState.value == .disabled else { guard recoveryKeyState.value == .disabled else {
MXLog.info("Resetting recovery key")
let key = try await encryption.resetRecoveryKey() let key = try await encryption.resetRecoveryKey()
return .success(key) return .success(key)
} }
MXLog.info("Enabling recovery")
var keyUploadErrored = false var keyUploadErrored = false
let recoveryKey = try await encryption.enableRecovery(waitForBackupsToUpload: false, progressListener: SecureBackupEnableRecoveryProgressListener { [weak self] state in let recoveryKey = try await encryption.enableRecovery(waitForBackupsToUpload: false, progressListener: SecureBackupEnableRecoveryProgressListener { [weak self] state in
guard let self else { return } guard let self else { return }
@ -106,38 +136,48 @@ class SecureBackupController: SecureBackupControllerProtocol {
case .done: case .done:
recoveryKeyStateSubject.send(.enabled) recoveryKeyStateSubject.send(.enabled)
case .roomKeyUploadError: case .roomKeyUploadError:
MXLog.error("Failed enabling recovery: room key upload error")
keyUploadErrored = true keyUploadErrored = true
} }
}) })
return keyUploadErrored ? .failure(.failedGeneratingRecoveryKey) : .success(recoveryKey) return keyUploadErrored ? .failure(.failedGeneratingRecoveryKey) : .success(recoveryKey)
} catch { } catch {
MXLog.error("Failed generating recovery key with error: \(error)")
return .failure(.failedGeneratingRecoveryKey) return .failure(.failedGeneratingRecoveryKey)
} }
} }
func confirmRecoveryKey(_ key: String) async -> Result<Void, SecureBackupControllerError> { func confirmRecoveryKey(_ key: String) async -> Result<Void, SecureBackupControllerError> {
do { do {
MXLog.info("Confirming recovery key")
try await encryption.recover(recoveryKey: key) try await encryption.recover(recoveryKey: key)
return .success(()) return .success(())
} catch { } catch {
MXLog.info("Failed confirming recovery key with error: \(error)")
return .failure(.failedConfirmingRecoveryKey) return .failure(.failedConfirmingRecoveryKey)
} }
} }
func isLastSession() async -> Result<Bool, SecureBackupControllerError> { func isLastSession() async -> Result<Bool, SecureBackupControllerError> {
do { do {
MXLog.info("Checking if last session")
return try await .success(encryption.isLastDevice()) return try await .success(encryption.isLastDevice())
} catch { } catch {
MXLog.error("Failed checking if last session with error: \(error)")
return .failure(.failedFetchingSessionState) return .failure(.failedFetchingSessionState)
} }
} }
func waitForKeyBackupUpload() async -> Result<Void, SecureBackupControllerError> { func waitForKeyBackupUpload() async -> Result<Void, SecureBackupControllerError> {
do { do {
MXLog.info("Waiting for backup upload steady state")
try await encryption.waitForBackupUploadSteadyState(progressListener: nil) try await encryption.waitForBackupUploadSteadyState(progressListener: nil)
return .success(()) return .success(())
} catch let error as SteadyStateError { } catch let error as SteadyStateError {
MXLog.error("Failed waiting for backup upload steady state with error: \(error)")
switch error { switch error {
case .BackupDisabled: case .BackupDisabled:
MXLog.error("Key backup disabled, continuing logout.") MXLog.error("Key backup disabled, continuing logout.")
@ -157,6 +197,7 @@ class SecureBackupController: SecureBackupControllerProtocol {
private func updateBackupStateFromRemote(retry: Bool = true) { private func updateBackupStateFromRemote(retry: Bool = true) {
remoteBackupStateTask = Task { remoteBackupStateTask = Task {
do { do {
MXLog.info("Checking if backup exists on the server")
let backupExists = try await self.encryption.backupExistsOnServer() let backupExists = try await self.encryption.backupExistsOnServer()
if Task.isCancelled { if Task.isCancelled {
@ -164,7 +205,7 @@ class SecureBackupController: SecureBackupControllerProtocol {
} }
if !backupExists { if !backupExists {
keyBackupStateSubject.send(.disabled) keyBackupStateSubject.send(.unknown)
} }
} catch { } catch {
MXLog.error("Failed retrieving remote backup state with error: \(error)") MXLog.error("Failed retrieving remote backup state with error: \(error)")
@ -212,24 +253,3 @@ private final class SecureBackupEnableRecoveryProgressListener: EnableRecoveryPr
onUpdateClosure(status) onUpdateClosure(status)
} }
} }
extension BackupState {
var keyBackupState: SecureBackupKeyBackupState {
switch self {
case .unknown:
return .unknown
case .creating:
return .enabling
case .enabling:
return .enabling
case .resuming:
return .enabled
case .enabled:
return .enabled
case .downloading:
return .enabled
case .disabling:
return .disabling
}
}
}

View File

@ -34,7 +34,6 @@ enum SecureBackupKeyBackupState {
case enabling case enabling
case enabled case enabled
case disabling case disabling
case disabled
} }
enum SecureBackupControllerError: Error { enum SecureBackupControllerError: Error {