diff --git a/Vault/Sources/VaultFeed/Presentation/Backup/BackupKeyChangeViewModel.swift b/Vault/Sources/VaultFeed/Presentation/Backup/BackupKeyChangeViewModel.swift index 94bb65503..ae98262cd 100644 --- a/Vault/Sources/VaultFeed/Presentation/Backup/BackupKeyChangeViewModel.swift +++ b/Vault/Sources/VaultFeed/Presentation/Backup/BackupKeyChangeViewModel.swift @@ -64,6 +64,10 @@ public final class BackupKeyChangeViewModel { public func didDisappear() { permissionState = .undetermined + // Don't retain the plaintext password beyond the lifetime of the + // screen that collected it. + newlyEnteredPassword = "" + newlyEnteredPasswordConfirm = "" } private struct PasswordConfirmError: Error {} @@ -79,16 +83,26 @@ public final class BackupKeyChangeViewModel { let createdBackupPassword = try await Task.background { try self.encryptionKeyDeriver.createEncryptionKey(password: password) } + // The KDF body is synchronous, so cancellation cannot + // interrupt it mid-derivation — make it authoritative here, + // before the derived key replaces the stored password. + try Task.checkCancellation() try await dataModel.store(backupPassword: createdBackupPassword) newPassword = .success newlyEnteredPassword = "" newlyEnteredPasswordConfirm = "" } catch is PasswordConfirmError { + // Keep the entered passwords: the user is mid-correction and + // the view is still frontmost. newPassword = .passwordConfirmError } catch is CancellationError { newPassword = .keygenCancelled + newlyEnteredPassword = "" + newlyEnteredPasswordConfirm = "" } catch { newPassword = .keygenError + newlyEnteredPassword = "" + newlyEnteredPasswordConfirm = "" } } diff --git a/Vault/Sources/VaultiOS/Views/Backup/BackupKeyChangeView.swift b/Vault/Sources/VaultiOS/Views/Backup/BackupKeyChangeView.swift index 20dd957bc..4251c327f 100644 --- a/Vault/Sources/VaultiOS/Views/Backup/BackupKeyChangeView.swift +++ b/Vault/Sources/VaultiOS/Views/Backup/BackupKeyChangeView.swift @@ -34,12 +34,18 @@ struct BackupKeyChangeView: View { await viewModel.onAppear() } .onDisappear { + // A dismissed view must never complete the key change in the + // background: cancel any in-flight keygen before resetting. + keyGenerationTask?.cancel() viewModel.didDisappear() } .toolbar { switch viewModel.newPassword { case .initial, .creating, .keygenCancelled, .keygenError, .passwordConfirmError: ToolbarItem(placement: .cancellationAction) { + // Deliberately enabled while the keygen runs: with + // interactive dismissal disabled, this is the only + // escape hatch from the up-to-3-minute derivation. Button { keyGenerationTask?.cancel() dismiss() @@ -47,7 +53,6 @@ struct BackupKeyChangeView: View { Text("Cancel") .tint(.red) } - .disabled(viewModel.newPassword.isLoading) } case .success: ToolbarItem(placement: .confirmationAction) { diff --git a/Vault/Tests/VaultFeedTests/Presentation/BackupKeyChangeViewModelTests.swift b/Vault/Tests/VaultFeedTests/Presentation/BackupKeyChangeViewModelTests.swift index 6f8437c7b..d8c74be6b 100644 --- a/Vault/Tests/VaultFeedTests/Presentation/BackupKeyChangeViewModelTests.swift +++ b/Vault/Tests/VaultFeedTests/Presentation/BackupKeyChangeViewModelTests.swift @@ -110,6 +110,86 @@ struct BackupKeyChangeViewModelTests { #expect(sut.newlyEnteredPassword == "") #expect(sut.newlyEnteredPasswordConfirm == "") } + + @Test + func saveEnteredPassword_cancelledBeforeStore_setsKeygenCancelledAndDoesNotStore() async { + let store = BackupPasswordStoreMock() + let dataModel = anyVaultDataModel(backupPasswordStore: store) + let sut = makeSUT(dataModel: dataModel) + + sut.newlyEnteredPassword = "hello" + sut.newlyEnteredPasswordConfirm = "hello" + + let task = Task { await sut.saveEnteredPassword() } + // Cancel before the task body has had a chance to run: the save + // must observe the cancellation and never replace the stored + // backup password. + task.cancel() + await task.value + + #expect(sut.newPassword == .keygenCancelled) + #expect(store.setCallCount == 0) + } + + @Test + func saveEnteredPassword_cancelled_clearsEnteredPasswords() async { + let sut = makeSUT() + + sut.newlyEnteredPassword = "hello" + sut.newlyEnteredPasswordConfirm = "hello" + + let task = Task { await sut.saveEnteredPassword() } + task.cancel() + await task.value + + #expect(sut.newlyEnteredPassword == "") + #expect(sut.newlyEnteredPasswordConfirm == "") + } + + @Test + func saveEnteredPassword_keygenError_clearsEnteredPasswords() async { + let deriverFactory = VaultKeyDeriverFactoryMock() + deriverFactory.makeVaultBackupKeyDeriverHandler = { + VaultKeyDeriver(deriver: KeyDeriverErroring(), signature: .testing) + } + let sut = makeSUT(deriverFactory: deriverFactory) + + sut.newlyEnteredPassword = "hello" + sut.newlyEnteredPasswordConfirm = "hello" + + await sut.saveEnteredPassword() + + #expect(sut.newlyEnteredPassword == "") + #expect(sut.newlyEnteredPasswordConfirm == "") + } + + @Test + func saveEnteredPassword_passwordConfirmError_retainsEnteredPasswords() async { + let sut = makeSUT() + + sut.newlyEnteredPassword = "hello" + sut.newlyEnteredPasswordConfirm = "world" + + await sut.saveEnteredPassword() + + // The user is mid-correction with the view still frontmost, so + // the entered text deliberately survives this error. + #expect(sut.newlyEnteredPassword == "hello") + #expect(sut.newlyEnteredPasswordConfirm == "world") + } + + @Test + func didDisappear_clearsEnteredPasswords() { + let sut = makeSUT() + + sut.newlyEnteredPassword = "hello" + sut.newlyEnteredPasswordConfirm = "hello" + + sut.didDisappear() + + #expect(sut.newlyEnteredPassword == "") + #expect(sut.newlyEnteredPasswordConfirm == "") + } } // MARK: - Helpers diff --git a/Vault/Tests/VaultiOSTests/BackupKeyChangeViewSnapshotTests.swift b/Vault/Tests/VaultiOSTests/BackupKeyChangeViewSnapshotTests.swift index b08ed8a5d..d0fa1ca01 100644 --- a/Vault/Tests/VaultiOSTests/BackupKeyChangeViewSnapshotTests.swift +++ b/Vault/Tests/VaultiOSTests/BackupKeyChangeViewSnapshotTests.swift @@ -1,7 +1,10 @@ +import CryptoEngine import Foundation +import FoundationExtensions import SwiftUI import TestHelpers import Testing +import VaultKeygen import VaultSettings @testable import VaultFeed @testable import VaultiOS @@ -23,6 +26,43 @@ final class BackupKeyChangeViewSnapshotTests { return BackupKeyChangeView(viewModel: viewModel) } } + + /// The Cancel button must render enabled while the keygen runs — it + /// is the only escape hatch from the up-to-3-minute derivation, since + /// interactive dismissal is disabled during `.creating`. + @Test + func layoutCreatingState() async { + for colorScheme in [ColorScheme.light, .dark] { + let deriver = BlockingKeyDeriver() + let viewModel = makeViewModel(deriver: deriver) + viewModel.permissionState = .allowed + viewModel.newlyEnteredPassword = "password" + viewModel.newlyEnteredPasswordConfirm = "password" + + let keygen = Task { await viewModel.saveEnteredPassword() } + while viewModel.newPassword != .creating { + await Task.yield() + } + + // Wrapped in a NavigationStack so the toolbar renders: the + // point of this snapshot is the enabled Cancel button. + let snapshottingView = NavigationStack { BackupKeyChangeView(viewModel: viewModel) } + .dynamicTypeSize(.medium) + .preferredColorScheme(colorScheme) + .framedForTest() + .environment(makePasteboard()) + .environment(DeviceAuthenticationService(policy: DeviceAuthenticationPolicyAlwaysAllow())) + assertSnapshot( + of: snapshottingView, + as: .image, + named: "\(colorScheme)_medium", + ) + + keygen.cancel() + deriver.release() + await keygen.value + } + } } // MARK: - Helpers @@ -36,6 +76,38 @@ extension BackupKeyChangeViewSnapshotTests { ) } + private func makeViewModel(deriver: BlockingKeyDeriver) -> BackupKeyChangeViewModel { + let deriverFactory = VaultKeyDeriverFactoryMock() + deriverFactory.makeVaultBackupKeyDeriverHandler = { + VaultKeyDeriver(deriver: deriver, signature: .testing) + } + return BackupKeyChangeViewModel( + dataModel: anyVaultDataModel(), + authenticationService: DeviceAuthenticationService(policy: DeviceAuthenticationPolicyAlwaysAllow()), + deriverFactory: deriverFactory, + ) + } + + /// Blocks key derivation until `release()` so tests can pin the + /// view in the `.creating` state. + // swiftlint:disable:next no_unchecked_sendable + private final class BlockingKeyDeriver: KeyDeriver, @unchecked Sendable { + private let semaphore = DispatchSemaphore(value: 0) + + var uniqueAlgorithmIdentifier: String { + "blocking" + } + + func key(password _: Data, salt _: Data) throws -> KeyData<32> { + semaphore.wait() + return .zero() + } + + func release() { + semaphore.signal() + } + } + /// Builds a fresh view for every scenario. /// /// The view resets `permissionState` to `.undetermined` in `onDisappear`, so sharing one view — diff --git a/Vault/Tests/VaultiOSTests/__Snapshots__/BackupKeyChangeViewSnapshotTests/layoutCreatingState.dark_medium.png b/Vault/Tests/VaultiOSTests/__Snapshots__/BackupKeyChangeViewSnapshotTests/layoutCreatingState.dark_medium.png new file mode 100644 index 000000000..980e10544 Binary files /dev/null and b/Vault/Tests/VaultiOSTests/__Snapshots__/BackupKeyChangeViewSnapshotTests/layoutCreatingState.dark_medium.png differ diff --git a/Vault/Tests/VaultiOSTests/__Snapshots__/BackupKeyChangeViewSnapshotTests/layoutCreatingState.light_medium.png b/Vault/Tests/VaultiOSTests/__Snapshots__/BackupKeyChangeViewSnapshotTests/layoutCreatingState.light_medium.png new file mode 100644 index 000000000..980e10544 Binary files /dev/null and b/Vault/Tests/VaultiOSTests/__Snapshots__/BackupKeyChangeViewSnapshotTests/layoutCreatingState.light_medium.png differ