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.
2026-11-02 12:07:55 +00:00 · 2026-07-01 10:13:18 +01:00
parent 7097ceb910
commit 123661537a
38 changed files with 296 additions and 21 deletions
@@ -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)
@@ -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<Bool, Never>) {
+        // 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.
@@ -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)
@@ -57,6 +57,7 @@ enum A11yIdentifiers {
    }
    
    struct AppLockScreen {
+        let cancel = "app_lock-cancel"
        func numpad(_ digit: Int) -> String {
            "app_lock-numpad_\(digit)"
        }
@@ -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)
@@ -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
 }
@@ -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)
        }
    }
    
@@ -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")
    }
 }
@@ -11,6 +11,7 @@ import SwiftUI
 enum LinkNewDeviceScreenCoordinatorAction {
    case linkMobileDevice(LinkNewDeviceService.LinkMobileProgressPublisher)
    case linkDesktopComputer
+    case verifyWithAppLockPIN(CheckedContinuation<Bool, Never>)
    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)
            }
@@ -10,6 +10,7 @@ import Foundation
 enum LinkNewDeviceScreenViewModelAction {
    case linkMobileDevice(LinkNewDeviceService.LinkMobileProgressPublisher)
    case linkDesktopComputer
+    case verifyWithAppLockPIN(CheckedContinuation<Bool, Never>)
    case dismiss
 }

@@ -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)
        
@@ -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 {
@@ -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<Int, Never> { 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<Int, Never> { get }
 }

+// MARK: - Mocks
+
 // sourcery: AutoMockable
 extension AppLockServiceProtocol { }

@@ -775,16 +775,30 @@ class MockScreen: Identifiable {
            coordinator.start()
            
            return navigationStackCoordinator
-        case .linkNewDevice:
+        case .linkNewDevice, .linkNewDeviceWithAppLockPIN:
            let linkMobileProgressSubject: CurrentValueSubject<LinkNewDeviceService.LinkMobileProgress, QRCodeLoginError> = .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
                    }
                }
@@ -26,6 +26,7 @@ enum UITestsScreenIdentifier: String {
    case encryptionSettingsOutOfSync
    case encryptionReset
    case linkNewDevice
+    case linkNewDeviceWithAppLockPIN
    case roomLayoutBottom
    case roomLayoutMiddle
    case roomLayoutTop