From 123661537a5c123edd3156cb13a88ae52cb23c8a Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Wed, 1 Jul 2026 10:13:18 +0100 Subject: [PATCH] Make LinkNewDeviceScreen fall back to the App Lock PIN code when there's no passcode. (#5794) * Fall back to the AppLock PIN code when there's no passcode. * Add extra tests on the LinkNewDeviceFlowCoordinator. * Misc tweaks for clarification. * Regenerate snapshots. --- .../AppLockFlowCoordinator.swift | 2 + .../LinkNewDeviceFlowCoordinator.swift | 31 ++++++++++ .../SettingsFlowCoordinator.swift | 2 + .../Other/AccessibilityIdentifiers.swift | 1 + .../AppLockScreenCoordinator.swift | 8 ++- .../AppLockScreen/AppLockScreenModels.swift | 14 +++++ .../AppLockScreenViewModel.swift | 6 +- .../AppLockScreen/View/AppLockScreen.swift | 20 +++++++ .../LinkNewDeviceScreenCoordinator.swift | 3 + .../LinkNewDeviceScreenModels.swift | 1 + .../LinkNewDeviceScreenViewModel.swift | 13 +++- .../Services/AppLock/AppLockService.swift | 5 ++ .../AppLock/AppLockServiceProtocol.swift | 14 +++-- .../UITests/UITestsAppCoordinator.swift | 20 ++++++- .../UITests/UITestsScreenIdentifier.swift | 1 + ...ng => appLockScreen.Unlock-iPad-en-GB.png} | 0 ...g => appLockScreen.Unlock-iPad-pseudo.png} | 0 ... => appLockScreen.Unlock-iPhone-en-GB.png} | 0 ...=> appLockScreen.Unlock-iPhone-pseudo.png} | 0 .../appLockScreen.Verify-iPad-en-GB.png | 3 + .../appLockScreen.Verify-iPad-pseudo.png | 3 + .../appLockScreen.Verify-iPhone-en-GB.png | 3 + .../appLockScreen.Verify-iPhone-pseudo.png | 3 + UITests/Sources/LinkNewDeviceTests.swift | 56 ++++++++++++++++++ ...estAppLockPINVerification-iPad-en-GB-1.png | 3 + ...estAppLockPINVerification-iPad-en-GB-2.png | 3 + ...estAppLockPINVerification-iPad-en-GB-4.png | 3 + ...tAppLockPINVerification-iPhone-en-GB-1.png | 3 + ...tAppLockPINVerification-iPhone-en-GB-2.png | 3 + ...tAppLockPINVerification-iPhone-en-GB-4.png | 3 + ...NewDevice.testForceLogout-iPad-en-GB-4.png | 3 + ...NewDevice.testForceLogout-iPad-en-GB-5.png | 3 + ...NewDevice.testForceLogout-iPad-en-GB-6.png | 3 + ...wDevice.testForceLogout-iPhone-en-GB-4.png | 3 + ...wDevice.testForceLogout-iPhone-en-GB-5.png | 3 + ...wDevice.testForceLogout-iPhone-en-GB-6.png | 3 + .../AppLock/AppLockScreenViewModelTests.swift | 13 ++++ .../LinkNewDeviceScreenViewModelTests.swift | 59 ++++++++++++++++--- 38 files changed, 296 insertions(+), 21 deletions(-) rename PreviewTests/Sources/__Snapshots__/PreviewTests/{appLockScreen.iPad-en-GB-0.png => appLockScreen.Unlock-iPad-en-GB.png} (100%) rename PreviewTests/Sources/__Snapshots__/PreviewTests/{appLockScreen.iPad-pseudo-0.png => appLockScreen.Unlock-iPad-pseudo.png} (100%) rename PreviewTests/Sources/__Snapshots__/PreviewTests/{appLockScreen.iPhone-en-GB-0.png => appLockScreen.Unlock-iPhone-en-GB.png} (100%) rename PreviewTests/Sources/__Snapshots__/PreviewTests/{appLockScreen.iPhone-pseudo-0.png => appLockScreen.Unlock-iPhone-pseudo.png} (100%) create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPad-en-GB.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPad-pseudo.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPhone-en-GB.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPhone-pseudo.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-1.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-2.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-4.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-1.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-2.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-4.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-4.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-5.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-6.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-4.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-5.png create mode 100644 UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-6.png diff --git a/ElementX/Sources/FlowCoordinators/AppLockFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/AppLockFlowCoordinator.swift index 9a1dd93fe..f910f8474 100644 --- a/ElementX/Sources/FlowCoordinators/AppLockFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/AppLockFlowCoordinator.swift @@ -242,6 +242,8 @@ class AppLockFlowCoordinator: CoordinatorProtocol { stateMachine.tryEvent(.didUnlockWithPIN) case .forceLogout: stateMachine.tryEvent(.forceLogout) + case .cancelVerifyDeviceOwner: + fatalError("Cancelling is not allowed when unlocking the app.") } } .store(in: &cancellables) diff --git a/ElementX/Sources/FlowCoordinators/LinkNewDeviceFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/LinkNewDeviceFlowCoordinator.swift index 304b9f23e..2f6381ea8 100644 --- a/ElementX/Sources/FlowCoordinators/LinkNewDeviceFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/LinkNewDeviceFlowCoordinator.swift @@ -10,6 +10,8 @@ import Foundation enum LinkNewDeviceFlowCoordinatorAction { case requestOAuthAuthorisation(URL, OAuthAccountSettingsPresenter.Continuation) + /// The user failed to remember their App Lock PIN. + case forceLogout case dismiss } @@ -58,6 +60,8 @@ class LinkNewDeviceFlowCoordinator: FlowCoordinatorProtocol { presentQRCodeScreen(mode: .linkMobile(progressPublisher, flowParameters.userSession.clientProxy)) case .linkDesktopComputer: presentQRCodeScreen(mode: .linkDesktop(flowParameters.userSession.clientProxy.linkNewDeviceService())) + case .verifyWithAppLockPIN(let continuation): + presentAppLockScreen(continuation: continuation) case .dismiss: actionsSubject.send(.dismiss) } @@ -67,6 +71,33 @@ class LinkNewDeviceFlowCoordinator: FlowCoordinatorProtocol { navigationStackCoordinator.setRootCoordinator(coordinator) } + private func presentAppLockScreen(continuation: CheckedContinuation) { + // We can use the AppLockScreen as a fallback here. It might seem weird but in the end it only calls + // AppLockService.unlock which is a no-op when the app is already unlocked. + let stackCoordinator = NavigationStackCoordinator() + let coordinator = AppLockScreenCoordinator(parameters: .init(appLockService: appLockService, mode: .verifyDeviceOwner)) + + coordinator.actions + .sink { [weak self] action in + guard let self else { return } + + switch action { + case .appUnlocked: + continuation.resume(returning: true) + navigationStackCoordinator.setFullScreenCoverCoordinator(nil) + case .cancelVerifyDeviceOwner: + continuation.resume(returning: false) + navigationStackCoordinator.setFullScreenCoverCoordinator(nil) + case .forceLogout: + actionsSubject.send(.forceLogout) + } + } + .store(in: &cancellables) + + stackCoordinator.setRootCoordinator(coordinator) + navigationStackCoordinator.setFullScreenCoverCoordinator(stackCoordinator) + } + private func presentQRCodeScreen(mode: QRCodeLoginScreenMode) { let coordinator = QRCodeLoginScreenCoordinator(parameters: .init(mode: mode, canSignInManually: false, // No need to worry about this when linking a device. diff --git a/ElementX/Sources/FlowCoordinators/SettingsFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/SettingsFlowCoordinator.swift index c9c15f624..07987d725 100644 --- a/ElementX/Sources/FlowCoordinators/SettingsFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/SettingsFlowCoordinator.swift @@ -192,6 +192,8 @@ class SettingsFlowCoordinator: FlowCoordinatorProtocol { navigationStackCoordinator.setSheetCoordinator(nil) case .requestOAuthAuthorisation(let url, let continuation): presentAccountManagementURL(url, continuation: continuation) + case .forceLogout: + actionsSubject.send(.forceLogout) } } .store(in: &cancellables) diff --git a/ElementX/Sources/Other/AccessibilityIdentifiers.swift b/ElementX/Sources/Other/AccessibilityIdentifiers.swift index 447781c62..5129d513e 100644 --- a/ElementX/Sources/Other/AccessibilityIdentifiers.swift +++ b/ElementX/Sources/Other/AccessibilityIdentifiers.swift @@ -57,6 +57,7 @@ enum A11yIdentifiers { } struct AppLockScreen { + let cancel = "app_lock-cancel" func numpad(_ digit: Int) -> String { "app_lock-numpad_\(digit)" } diff --git a/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenCoordinator.swift b/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenCoordinator.swift index 9041e1942..73a22cf8e 100644 --- a/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenCoordinator.swift +++ b/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenCoordinator.swift @@ -12,6 +12,8 @@ import SwiftUI struct AppLockScreenCoordinatorParameters { /// The service used to unlock the app. let appLockService: AppLockServiceProtocol + /// The context in which the screen is being shown. + var mode: AppLockScreenMode = .appUnlock } enum AppLockScreenCoordinatorAction { @@ -19,6 +21,8 @@ enum AppLockScreenCoordinatorAction { case appUnlocked /// The user failed to unlock the app (or forgot their PIN). case forceLogout + /// The user cancelled device owner verification. Only sent in the `.verifyDeviceOwner` mode. + case cancelVerifyDeviceOwner } final class AppLockScreenCoordinator: CoordinatorProtocol { @@ -31,7 +35,7 @@ final class AppLockScreenCoordinator: CoordinatorProtocol { } init(parameters: AppLockScreenCoordinatorParameters) { - viewModel = AppLockScreenViewModel(appLockService: parameters.appLockService) + viewModel = AppLockScreenViewModel(appLockService: parameters.appLockService, mode: parameters.mode) } func start() { @@ -44,6 +48,8 @@ final class AppLockScreenCoordinator: CoordinatorProtocol { self.actionsSubject.send(.appUnlocked) case .forceLogout: self.actionsSubject.send(.forceLogout) + case .cancelVerifyDeviceOwner: + self.actionsSubject.send(.cancelVerifyDeviceOwner) } } .store(in: &cancellables) diff --git a/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenModels.swift b/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenModels.swift index 17b8fc16b..758a3569c 100644 --- a/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenModels.swift +++ b/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenModels.swift @@ -13,9 +13,21 @@ enum AppLockScreenViewModelAction { case appUnlocked /// The user failed to unlock the app (or forgot their PIN). case forceLogout + /// The user cancelled device owner verification. Only sent in the `.verifyDeviceOwner` mode. + case cancelVerifyDeviceOwner +} + +enum AppLockScreenMode { + /// Unlocking the app on launch/foregrounding. + case appUnlock + /// Verifying that the user is the device owner to gate a sensitive action. + case verifyDeviceOwner } struct AppLockScreenViewState: BindableState { + /// The context in which the screen is being shown. + var mode: AppLockScreenMode = .appUnlock + /// The number of attempts allowed to unlock the app. let maximumAttempts = 3 @@ -64,4 +76,6 @@ enum AppLockScreenViewAction { case clearPINCode /// The user didn't heed the warnings and can't remember their PIN. case forgotPIN + /// The user cancelled device owner verification. + case cancelVerifyDeviceOwner } diff --git a/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenViewModel.swift b/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenViewModel.swift index 68e595926..15b04cbc8 100644 --- a/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenViewModel.swift +++ b/ElementX/Sources/Screens/AppLock/AppLockScreen/AppLockScreenViewModel.swift @@ -19,10 +19,10 @@ class AppLockScreenViewModel: AppLockScreenViewModelType, AppLockScreenViewModel actionsSubject.eraseToAnyPublisher() } - init(appLockService: AppLockServiceProtocol) { + init(appLockService: AppLockServiceProtocol, mode: AppLockScreenMode = .appUnlock) { self.appLockService = appLockService - super.init(initialViewState: AppLockScreenViewState(bindings: .init())) + super.init(initialViewState: AppLockScreenViewState(mode: mode, bindings: .init())) appLockService.numberOfPINAttempts .weakAssign(to: \.state.numberOfPINAttempts, on: self) @@ -51,6 +51,8 @@ class AppLockScreenViewModel: AppLockScreenViewModelType, AppLockScreenViewModel state.bindings.pinCode = "" case .forgotPIN: handleForgotPIN() + case .cancelVerifyDeviceOwner: + actionsSubject.send(.cancelVerifyDeviceOwner) } } diff --git a/ElementX/Sources/Screens/AppLock/AppLockScreen/View/AppLockScreen.swift b/ElementX/Sources/Screens/AppLock/AppLockScreen/View/AppLockScreen.swift index ee854e9ef..43d8e4fe7 100644 --- a/ElementX/Sources/Screens/AppLock/AppLockScreen/View/AppLockScreen.swift +++ b/ElementX/Sources/Screens/AppLock/AppLockScreen/View/AppLockScreen.swift @@ -54,6 +54,19 @@ struct AppLockScreen: View { .animation(.elementDefault, value: context.viewState.forcedLogoutIndicator) } .alert(item: $context.alertInfo) + .toolbar { toolbar } + } + + @ToolbarContentBuilder + var toolbar: some ToolbarContent { + if context.viewState.mode == .verifyDeviceOwner { + ToolbarItem(placement: .cancellationAction) { + ToolbarButton(role: .cancel) { + context.send(viewAction: .cancelVerifyDeviceOwner) + } + .accessibilityIdentifier(A11yIdentifiers.appLockScreen.cancel) + } + } } var header: some View { @@ -114,10 +127,17 @@ struct AppLockScreen: View { struct AppLockScreen_Previews: PreviewProvider, TestablePreview { static let viewModel = AppLockScreenViewModel(appLockService: AppLockServiceMock.mock()) + static let verifyViewModel = AppLockScreenViewModel(appLockService: AppLockServiceMock.mock(), mode: .verifyDeviceOwner) static var previews: some View { ElementNavigationStack { AppLockScreen(context: viewModel.context) } + .previewDisplayName("Unlock") + + ElementNavigationStack { + AppLockScreen(context: verifyViewModel.context) + } + .previewDisplayName("Verify") } } diff --git a/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenCoordinator.swift b/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenCoordinator.swift index 9370d0d3b..baac325c0 100644 --- a/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenCoordinator.swift +++ b/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenCoordinator.swift @@ -11,6 +11,7 @@ import SwiftUI enum LinkNewDeviceScreenCoordinatorAction { case linkMobileDevice(LinkNewDeviceService.LinkMobileProgressPublisher) case linkDesktopComputer + case verifyWithAppLockPIN(CheckedContinuation) case dismiss } @@ -47,6 +48,8 @@ final class LinkNewDeviceScreenCoordinator: CoordinatorProtocol { actionsSubject.send(.linkMobileDevice(progressPublisher)) case .linkDesktopComputer: actionsSubject.send(.linkDesktopComputer) + case .verifyWithAppLockPIN(let continuation): + actionsSubject.send(.verifyWithAppLockPIN(continuation)) case .dismiss: actionsSubject.send(.dismiss) } diff --git a/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenModels.swift b/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenModels.swift index e30e98e3d..2e9ca0b33 100644 --- a/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenModels.swift +++ b/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenModels.swift @@ -10,6 +10,7 @@ import Foundation enum LinkNewDeviceScreenViewModelAction { case linkMobileDevice(LinkNewDeviceService.LinkMobileProgressPublisher) case linkDesktopComputer + case verifyWithAppLockPIN(CheckedContinuation) case dismiss } diff --git a/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenViewModel.swift b/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenViewModel.swift index 7f7451663..1ad200548 100644 --- a/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenViewModel.swift +++ b/ElementX/Sources/Screens/LinkNewDeviceScreen/LinkNewDeviceScreenViewModel.swift @@ -72,8 +72,10 @@ class LinkNewDeviceScreenViewModel: LinkNewDeviceScreenViewModelType, LinkNewDev case .verified, .unavailable: break case .appLockPINRequired: - // Follow-up PR: present the App Lock PIN screen to verify the device owner. - return + guard await verifyWithAppLockPIN() else { + state.mode = .readyToLink(.idle) + return + } case .cancelled, .unverified: state.mode = .readyToLink(.idle) return @@ -92,6 +94,13 @@ class LinkNewDeviceScreenViewModel: LinkNewDeviceScreenViewModelType, LinkNewDev } } + /// Asks the coordinator to verify the device owner with the App Lock PIN, suspending until it has a result. + private func verifyWithAppLockPIN() async -> Bool { + await withCheckedContinuation { continuation in + actionsSubject.send(.verifyWithAppLockPIN(continuation)) + } + } + private func linkMobileDevice() async { state.mode = .readyToLink(.generatingCode) diff --git a/ElementX/Sources/Services/AppLock/AppLockService.swift b/ElementX/Sources/Services/AppLock/AppLockService.swift index 2a55bd8a9..8b5c91b61 100644 --- a/ElementX/Sources/Services/AppLock/AppLockService.swift +++ b/ElementX/Sources/Services/AppLock/AppLockService.swift @@ -16,7 +16,12 @@ class AppLockService: AppLockServiceProtocol { private let context: LAContext private let timer: AppLockTimer + /// Only use Biometrics to unlock the app, we want to fall back to the App Lock PIN, not the device's passcode. private let unlockPolicy: LAPolicy = .deviceOwnerAuthenticationWithBiometrics + /// Use biometrics *or* the device's passcode to verify the device owner. Available regardless of whether biometrics are enrolled. + /// + /// **Note:** Verifying the device owner is used to gate sensitive actions, it isn't related to unlocking the app. + /// Maybe we should rename this service in the future. private let deviceOwnerPolicy: LAPolicy = .deviceOwnerAuthentication var isMandatory: Bool { diff --git a/ElementX/Sources/Services/AppLock/AppLockServiceProtocol.swift b/ElementX/Sources/Services/AppLock/AppLockServiceProtocol.swift index 90c123501..034cf85d4 100644 --- a/ElementX/Sources/Services/AppLock/AppLockServiceProtocol.swift +++ b/ElementX/Sources/Services/AppLock/AppLockServiceProtocol.swift @@ -88,19 +88,23 @@ protocol AppLockServiceProtocol: AnyObject { /// Attempt to unlock the app using FaceID or TouchID. func unlockWithBiometrics() async -> AppLockServiceBiometricResult + /// The number of attempts the user had made to unlock with a PIN code. + /// + /// Note: We don't track the biometric attempts as LAContext does that automatically. + var numberOfPINAttempts: AnyPublisher { get } + + // MARK: Device Owner Verification + /// Verifies the device owner via the device passcode/biometrics, e.g. to gate a sensitive action. /// /// This is a standalone identity check: it does **not** unlock the app or touch any App Lock /// state (the grace-period timer or PIN attempt count). /// - Parameter reason: The localised reason shown in the system prompt. func verifyDeviceOwner(reason: String) async -> AppLockDeviceOwnerResult - - /// The number of attempts the user had made to unlock with a PIN code. - /// - /// Note: We don't track the biometric attempts as LAContext does that automatically. - var numberOfPINAttempts: AnyPublisher { get } } +// MARK: - Mocks + // sourcery: AutoMockable extension AppLockServiceProtocol { } diff --git a/ElementX/Sources/UITests/UITestsAppCoordinator.swift b/ElementX/Sources/UITests/UITestsAppCoordinator.swift index b46695a44..956e489e9 100644 --- a/ElementX/Sources/UITests/UITestsAppCoordinator.swift +++ b/ElementX/Sources/UITests/UITestsAppCoordinator.swift @@ -775,16 +775,30 @@ class MockScreen: Identifiable { coordinator.start() return navigationStackCoordinator - case .linkNewDevice: + case .linkNewDevice, .linkNewDeviceWithAppLockPIN: let linkMobileProgressSubject: CurrentValueSubject = .init(.qrReady(LinkNewDeviceServiceMock.mockQRCodeImage)) let linkNewDeviceService = LinkNewDeviceServiceMock(.init(linkMobileProgressPublisher: linkMobileProgressSubject.asCurrentValuePublisher())) let clientProxy = ClientProxyMock(.init()) clientProxy.linkNewDeviceServiceReturnValue = linkNewDeviceService + let keychainController = KeychainController(service: .tests, accessGroup: InfoPlistReader.main.keychainAccessGroupIdentifier) + keychainController.resetSecrets() + + let context = LAContextMock() + context.canEvaluatePolicyReturnValue = false // No device passcode in the simulator. + + let appLockService = AppLockService(keychainController: keychainController, appSettings: appSettings, context: context) + + // Set an App Lock PIN for the dedicated variant so linking requires it (and forced logout can be tested). + // Without a PIN the service is disabled, so verification is unavailable and linking proceeds. + if id == .linkNewDeviceWithAppLockPIN, case .failure = appLockService.setupPINCode("2023") { + fatalError("Failed to preset the PIN code.") + } + let navigationStackCoordinator = NavigationStackCoordinator() let flowCoordinator = LinkNewDeviceFlowCoordinator(navigationStackCoordinator: navigationStackCoordinator, - appLockService: AppLockServiceMock.mock(), + appLockService: appLockService, flowParameters: CommonFlowParameters(userSession: UserSessionMock(.init(clientProxy: clientProxy)), bugReportService: BugReportServiceMock(.init()), elementCallService: ElementCallServiceMock(.init()), @@ -804,7 +818,7 @@ class MockScreen: Identifiable { switch action { case .dismiss: navigationRootCoordinator.setSheetCoordinator(nil) - case .requestOAuthAuthorisation: + case .requestOAuthAuthorisation, .forceLogout: break } } diff --git a/ElementX/Sources/UITests/UITestsScreenIdentifier.swift b/ElementX/Sources/UITests/UITestsScreenIdentifier.swift index 85971e6d2..5bbe080a8 100644 --- a/ElementX/Sources/UITests/UITestsScreenIdentifier.swift +++ b/ElementX/Sources/UITests/UITestsScreenIdentifier.swift @@ -26,6 +26,7 @@ enum UITestsScreenIdentifier: String { case encryptionSettingsOutOfSync case encryptionReset case linkNewDevice + case linkNewDeviceWithAppLockPIN case roomLayoutBottom case roomLayoutMiddle case roomLayoutTop diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.iPad-en-GB-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Unlock-iPad-en-GB.png similarity index 100% rename from PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.iPad-en-GB-0.png rename to PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Unlock-iPad-en-GB.png diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.iPad-pseudo-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Unlock-iPad-pseudo.png similarity index 100% rename from PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.iPad-pseudo-0.png rename to PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Unlock-iPad-pseudo.png diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.iPhone-en-GB-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Unlock-iPhone-en-GB.png similarity index 100% rename from PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.iPhone-en-GB-0.png rename to PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Unlock-iPhone-en-GB.png diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.iPhone-pseudo-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Unlock-iPhone-pseudo.png similarity index 100% rename from PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.iPhone-pseudo-0.png rename to PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Unlock-iPhone-pseudo.png diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPad-en-GB.png new file mode 100644 index 000000000..46d828bc9 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPad-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2dca9ed5f2e5fe872b14ca4b80a75ca3639691424cec998a984452f5edc58beb +size 115003 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPad-pseudo.png new file mode 100644 index 000000000..0ba93d344 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPad-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:19a50e49aba3847ab76e11d82fa48856b4366484a158d50d0e9eb062700ce6a2 +size 123576 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPhone-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPhone-en-GB.png new file mode 100644 index 000000000..5ce5e915e --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPhone-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:9d1f81da13faa600035bccb951adf1d81ed9e2dc5e21cdc782a4fc623243e0dc +size 70283 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPhone-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPhone-pseudo.png new file mode 100644 index 000000000..a3fc7d368 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/appLockScreen.Verify-iPhone-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:9333374fd5b3affc6bdff106669b66b468b22b6705c157fe0569c308bb850c04 +size 77149 diff --git a/UITests/Sources/LinkNewDeviceTests.swift b/UITests/Sources/LinkNewDeviceTests.swift index a0055f027..7f7af9037 100644 --- a/UITests/Sources/LinkNewDeviceTests.swift +++ b/UITests/Sources/LinkNewDeviceTests.swift @@ -13,6 +13,9 @@ class LinkNewDeviceTests: XCTestCase { static let selectDevice = 1 static let linkMobileDevice = 2 static let linkDesktopComputer = 3 + static let appLockPIN = 4 + static let logoutAlert = 5 + static let forcedLogout = 6 static let dismissed = 99 } @@ -45,4 +48,57 @@ class LinkNewDeviceTests: XCTestCase { cancelButton.tap() try await app.assertScreenshot(step: Step.dismissed) } + + func testAppLockPINVerification() async throws { + // Root screen + let app = Application.launch(.linkNewDeviceWithAppLockPIN) + try await app.assertScreenshot(step: Step.selectDevice) + + // Linking requires verifying the device owner with the App Lock PIN. + let mobileDeviceButton = app.buttons[A11yIdentifiers.linkNewDeviceScreen.mobileDevice] + mobileDeviceButton.tap() + try await app.assertScreenshot(step: Step.appLockPIN) + + // Cancelling verification returns to the root screen. + app.buttons[A11yIdentifiers.appLockScreen.cancel].tap() + try await app.assertScreenshot(step: Step.selectDevice) + + // Entering the correct PIN continues to generate the QR code. + mobileDeviceButton.tap() + try await app.assertScreenshot(step: Step.appLockPIN) + enterPIN(app) + try await app.assertScreenshot(step: Step.linkMobileDevice) + } + + func testForceLogout() async throws { + // Root screen + let app = Application.launch(.linkNewDeviceWithAppLockPIN) + + // Linking requires verifying the device owner with the App Lock PIN. + app.buttons[A11yIdentifiers.linkNewDeviceScreen.mobileDevice].tap() + try await app.assertScreenshot(step: Step.appLockPIN) + + // Entering the wrong PIN three times signs the user out. + enterWrongPIN(app) + enterWrongPIN(app) + enterWrongPIN(app) + try await app.assertScreenshot(step: Step.logoutAlert) + app.alerts.element.buttons[A11yIdentifiers.alertInfo.primaryButton].firstMatch.tap() + try await app.assertScreenshot(step: Step.forcedLogout) + } + + // MARK: - Helpers + + private func enterPIN(_ app: XCUIApplication) { + app.buttons[A11yIdentifiers.appLockScreen.numpad(2)].tap() + app.buttons[A11yIdentifiers.appLockScreen.numpad(0)].tap() + app.buttons[A11yIdentifiers.appLockScreen.numpad(2)].tap() + app.buttons[A11yIdentifiers.appLockScreen.numpad(3)].tap() + } + + private func enterWrongPIN(_ app: XCUIApplication) { + for _ in 0..<4 { + app.buttons[A11yIdentifiers.appLockScreen.numpad(0)].tap() + } + } } diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-1.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-1.png new file mode 100644 index 000000000..2ff9b2201 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-1.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:86177b8a2db758a936a2a11a0f531bfb45511ca6c95b534d14946fd7543725e0 +size 158276 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-2.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-2.png new file mode 100644 index 000000000..2ecc4e484 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-2.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d73df700e470e40b07a5dfdcb250fec206c2ab5ace9203847a5b0f4126b89b82 +size 190553 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-4.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-4.png new file mode 100644 index 000000000..bb5ab35da --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPad-en-GB-4.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:14451bd62aa2ac5ddee3eab2db4aee40d82f5a8b05950f4d03417ef315650774 +size 111945 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-1.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-1.png new file mode 100644 index 000000000..3664ea9a9 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-1.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:a2abf3877cb363cae2d674fa3d11e059a62a60fc4a4e3eb8099f25c3187fe71e +size 115280 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-2.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-2.png new file mode 100644 index 000000000..7dd1045f7 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-2.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2c8b2704da62a90ce7a558f8d2a4f8d42e7622c439976432dbbe00beeded4d28 +size 164780 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-4.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-4.png new file mode 100644 index 000000000..268a781d6 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testAppLockPINVerification-iPhone-en-GB-4.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d4f7231a3b4291a370d0c2507c522012f4edeb9878cd9fbb468f36c91d671e99 +size 128890 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-4.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-4.png new file mode 100644 index 000000000..bb5ab35da --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-4.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:14451bd62aa2ac5ddee3eab2db4aee40d82f5a8b05950f4d03417ef315650774 +size 111945 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-5.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-5.png new file mode 100644 index 000000000..5166e7443 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-5.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6678652d0e5a48dd7cfa197d52ff161cdcad75108c183a2748eb37099aedcda5 +size 203737 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-6.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-6.png new file mode 100644 index 000000000..b807d878c --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPad-en-GB-6.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:aaee386193e4980a9ff49b4f06ab4f9885839153fadbca373b92182aff59e62e +size 123853 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-4.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-4.png new file mode 100644 index 000000000..268a781d6 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-4.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d4f7231a3b4291a370d0c2507c522012f4edeb9878cd9fbb468f36c91d671e99 +size 128890 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-5.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-5.png new file mode 100644 index 000000000..c6fba3592 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-5.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4fa4bb4e82f027e6a716ce28e49269718291b141f649bbd389dab45ecad16bc6 +size 280901 diff --git a/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-6.png b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-6.png new file mode 100644 index 000000000..0adef5009 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/linkNewDevice.testForceLogout-iPhone-en-GB-6.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:32951bca5d167c6ce1523037cd31404f223b0ba2214247e68fedf6371f1d2d70 +size 148086 diff --git a/UnitTests/Sources/AppLock/AppLockScreenViewModelTests.swift b/UnitTests/Sources/AppLock/AppLockScreenViewModelTests.swift index e673d7515..05d5efd7a 100644 --- a/UnitTests/Sources/AppLock/AppLockScreenViewModelTests.swift +++ b/UnitTests/Sources/AppLock/AppLockScreenViewModelTests.swift @@ -43,6 +43,19 @@ final class AppLockScreenViewModelTests { #expect(result == .appUnlocked) } + @Test + func cancelVerifyDeviceOwner() async throws { + // Given a screen shown to verify the device owner. + viewModel = AppLockScreenViewModel(appLockService: appLockService, mode: .verifyDeviceOwner) + + // When the user cancels. + let deferred = deferFulfillment(viewModel.actions) { $0 == .cancelVerifyDeviceOwner } + viewModel.context.send(viewAction: .cancelVerifyDeviceOwner) + + // Then the cancellation should be reported. + try await deferred.fulfill() + } + @Test func forgotPIN() async throws { // Given a fresh launch of the app. diff --git a/UnitTests/Sources/LinkNewDeviceScreenViewModelTests.swift b/UnitTests/Sources/LinkNewDeviceScreenViewModelTests.swift index 232ccc0ad..c87a400c2 100644 --- a/UnitTests/Sources/LinkNewDeviceScreenViewModelTests.swift +++ b/UnitTests/Sources/LinkNewDeviceScreenViewModelTests.swift @@ -91,8 +91,8 @@ class LinkNewDeviceScreenViewModelTests { } @Test - func linkingProceedsWhenVerificationUnavailable() async throws { - // Given a device with no passcode set, so device owner verification is unavailable. + func linkingWithoutDevicePasscodeOrAppLockPIN() async throws { + // Given a device with neither a passcode nor an App Lock PIN set (device owner verification is unavailable). let progressSubject = CurrentValueSubject(.starting) setupViewModel(linkMobileProgressPublisher: progressSubject.asCurrentValuePublisher(), deviceOwnerResult: .unavailable, @@ -109,20 +109,56 @@ class LinkNewDeviceScreenViewModelTests { } @Test - func appLockPINRequiredDoesNotLink() async throws { + func appLockPINRequiredRequestsVerification() async throws { // Given a device with no passcode but an App Lock PIN set, so PIN verification is required. setupViewModel(deviceOwnerResult: .appLockPINRequired, mode: .loading) - // When linking a mobile device, it should not proceed to generate a QR code. - // TODO: assert the App Lock PIN screen is presented once it's wired up. - let deferred = deferFailure(context.observe(\.viewState.mode), timeout: .seconds(1)) { $0 == .readyToLink(.generatingCode) } + // When linking a mobile device. + let deferred = deferFulfillment(viewModel.actionsPublisher) { $0.isVerifyWithAppLockPIN } context.send(viewAction: .linkMobileDevice) - try await deferred.fulfill() + let action = try await deferred.fulfill() - // Then verification should have been attempted but linking should not have started. + // Then it should request App Lock PIN verification rather than linking. #expect(appLockService.verifyDeviceOwnerReasonCalled) #expect(!linkNewDeviceService.linkMobileDeviceCalled) + + // When cancelling the verification. + guard case let .verifyWithAppLockPIN(continuation) = action else { + Issue.record("Unexpected action.") + return + } + let deferredIdle = deferFulfillment(context.observe(\.viewState.mode)) { $0 == .readyToLink(.idle) } + continuation.resume(returning: false) + + // Then it should return to idle without linking. + try await deferredIdle.fulfill() + #expect(!linkNewDeviceService.linkMobileDeviceCalled) + } + + @Test + func appLockPINVerifiedContinuesLinking() async throws { + // Given a device that requires App Lock PIN verification. + let progressSubject = CurrentValueSubject(.starting) + setupViewModel(linkMobileProgressPublisher: progressSubject.asCurrentValuePublisher(), + deviceOwnerResult: .appLockPINRequired, + mode: .loading) + + // When linking a mobile device and the PIN verification succeeds. + let deferred = deferFulfillment(viewModel.actionsPublisher) { $0.isVerifyWithAppLockPIN } + context.send(viewAction: .linkMobileDevice) + let action = try await deferred.fulfill() + guard case let .verifyWithAppLockPIN(continuation) = action else { + Issue.record("Unexpected action.") + return + } + + let deferredGenerating = deferFulfillment(context.observe(\.viewState.mode)) { $0 == .readyToLink(.generatingCode) } + continuation.resume(returning: true) + + // Then it should proceed to generate the QR code. + try await deferredGenerating.fulfill() + #expect(linkNewDeviceService.linkMobileDeviceCalled) } // MARK: - Verification failures @@ -259,4 +295,11 @@ private extension LinkNewDeviceScreenViewModelAction { default: false } } + + var isVerifyWithAppLockPIN: Bool { + switch self { + case .verifyWithAppLockPIN: true + default: false + } + } }