Add a state machine to AppLockFlowCoordinator. (#2166)

Fixes a bug when cancelling Face ID would loop back round to presenting Face ID.

ElementX/Sources/FlowCoordinators/AppLockFlowCoordinator.swift

@@ -15,6 +15,7 @@
 //
 
 import Combine
+import SwiftState
 import SwiftUI
 
 enum AppLockFlowCoordinatorAction: Equatable {
@@ -31,100 +32,193 @@ class AppLockFlowCoordinator: CoordinatorProtocol {
     let appLockService: AppLockServiceProtocol
     let navigationCoordinator: NavigationRootCoordinator
     
+    /// States the flow can find itself in
+    enum State: StateType {
+        /// The initial state before the app has launched.
+        case initial
+        /// The app is in the foreground and visible to the user.
+        case unlocked
+        /// The app has resigned active but is not yet in the background. This state
+        /// shows the placeholder, but doesn't require an unlock on becoming active.
+        case appObscured
+        /// The app is in the background.
+        case backgrounded
+        /// The app is presenting biometric unlock to the user.
+        case attemptingBiometricUnlock
+        /// Biometric unlock has completed but the system UI is still the active input.
+        /// Once the app becomes active again, it will trigger the next state.
+        case dismissingBiometricUnlock(AppLockServiceBiometricResult)
+        /// The app is presenting the unlock screen for PIN code entry.
+        case attemptingPINUnlock
+        /// The user failed to unlock the app (or forgot their PIN) and is being logged out.
+        case loggingOut
+    }
+
+    /// Events that can be triggered on the flow state machine
+    enum Event: EventType {
+        /// The app is resigning active (going into the app switcher, showing system UI like Face ID, permissions prompt etc).
+        case willResignActive
+        /// The app is now backgrounded and not visible to the user.
+        case didEnterBackground
+        /// The app is in the foreground and has been given focus.
+        case didBecomeActive
+        /// Biometric unlock has completed with the following result.
+        case didFinishBiometricUnlock(AppLockServiceBiometricResult)
+        /// The entered PIN code was accepted.
+        case didUnlockWithPIN
+        /// The user failed to unlock the app (or forgot their PIN).
+        case forceLogout
+        /// The service has been enabled.
+        case serviceEnabled
+        /// The service has been disabled.
+        case serviceDisabled
+    }
+    
+    private let stateMachine: StateMachine<State, Event>
+    
     /// A task used to await biometric unlock before showing the PIN screen.
     @CancellableTask private var unlockTask: Task<Void, Never>?
     private var cancellables: Set<AnyCancellable> = []
     
+    /// Whether or not biometric unlock should be attempted instead of asking for a PIN.
+    private var isBiometricUnlockAvailable: Bool {
+        appLockService.biometricUnlockEnabled && appLockService.biometricUnlockTrusted
+    }
+    
     private let actionsSubject: PassthroughSubject<AppLockFlowCoordinatorAction, Never> = .init()
     var actions: AnyPublisher<AppLockFlowCoordinatorAction, Never> {
         actionsSubject.eraseToAnyPublisher()
     }
     
-    init(appLockService: AppLockServiceProtocol,
+    init(initialState: State = .initial,
+         appLockService: AppLockServiceProtocol,
          navigationCoordinator: NavigationRootCoordinator,
          notificationCenter: NotificationCenter = .default) {
         self.appLockService = appLockService
         self.navigationCoordinator = navigationCoordinator
         
-        // Set the initial background state.
+        // Set the initial state and start with the placeholder screen as the root view.
+        stateMachine = .init(state: initialState)
         showPlaceholder()
         
         notificationCenter.publisher(for: UIApplication.willResignActiveNotification)
             .sink { [weak self] _ in
-                self?.applicationWillResignActive()
+                self?.stateMachine.tryEvent(.willResignActive)
             }
             .store(in: &cancellables)
         
         notificationCenter.publisher(for: UIApplication.didEnterBackgroundNotification)
             .sink { [weak self] _ in
-                self?.applicationDidEnterBackground()
+                self?.stateMachine.tryEvent(.didEnterBackground)
             }
             .store(in: &cancellables)
         
         notificationCenter.publisher(for: UIApplication.didBecomeActiveNotification)
             .sink { [weak self] _ in
-                self?.applicationDidBecomeActive()
+                self?.stateMachine.tryEvent(.didBecomeActive)
             }
             .store(in: &cancellables)
+        
+        appLockService.isEnabledPublisher
+            .sink { [weak self] isEnabled in
+                self?.stateMachine.tryEvent(isEnabled ? .serviceEnabled : .serviceDisabled)
+            }
+            .store(in: &cancellables)
+        
+        configureStateMachine()
     }
     
     func toPresentable() -> AnyView {
         AnyView(navigationCoordinator.toPresentable())
     }
     
-    // MARK: - App unlock
+    // MARK: - State machine
     
-    private func applicationWillResignActive() {
+    private func configureStateMachine() {
-        unlockTask = nil
+        stateMachine.addRouteMapping { [weak self] event, fromState, _ in
-        
+            guard let self, appLockService.isEnabled else { return fromState }
-        guard appLockService.isEnabled else { return }
-        showPlaceholder()
-    }
-    
-    private func applicationDidEnterBackground() {
-        guard appLockService.isEnabled else { return }
-        appLockService.applicationDidEnterBackground()
-        showPlaceholder() // Double call but just to be safe
-    }
-    
-    private func applicationDidBecomeActive() {
-        guard appLockService.isEnabled else { return }
-        
-        guard appLockService.computeNeedsUnlock(didBecomeActiveAt: .now) else {
-            // Reveal the app again if within the grace period.
-            actionsSubject.send(.unlockApp)
-            return
-        }
-        
-        // Show the relevant unlock mechanism.
-        unlockTask = Task { [weak self] in
-            guard let self else { return }
-            await startUnlockFlow()
-        }
-    }
-    
-    /// Runs the unlock flow, showing Touch ID/Face ID if available, transitioning to PIN unlock if it fails or isn't available.
-    private func startUnlockFlow() async {
-        if appLockService.biometricUnlockEnabled, appLockService.biometricUnlockTrusted {
-            showPlaceholder() // For the unlock background.
             
-            if await appLockService.unlockWithBiometrics(), UIApplication.shared.applicationState == .active {
+            switch (fromState, event) {
-                actionsSubject.send(.unlockApp)
+            case (.unlocked, .willResignActive):
-                return
+                return .appObscured
+            case (.appObscured, .didBecomeActive):
+                return .unlocked
+            case (_, .didEnterBackground):
+                return .backgrounded
+            case (.backgrounded, .didBecomeActive), (.initial, .didBecomeActive):
+                guard appLockService.computeNeedsUnlock(didBecomeActiveAt: .now) else { return .unlocked }
+                return isBiometricUnlockAvailable ? .attemptingBiometricUnlock : .attemptingPINUnlock
+            case (.attemptingBiometricUnlock, .didFinishBiometricUnlock(let result)):
+                return .dismissingBiometricUnlock(result) // Transitional state until the app becomes active again.
+            case (.dismissingBiometricUnlock(let result), .didBecomeActive):
+                return switch result {
+                case .unlocked: .unlocked
+                case .failed: .attemptingPINUnlock
+                case .interrupted: .attemptingBiometricUnlock
+                }
+            case (.attemptingPINUnlock, .didUnlockWithPIN):
+                return .unlocked
+            case (.attemptingPINUnlock, .forceLogout):
+                return .loggingOut
+            
+            // Transition to a valid state when enabling the service for the first time.
+            case (.initial, .serviceEnabled):
+                return .unlocked
+            // Transition to a valid state once the service is disabled following a forced logout.
+            case (.loggingOut, .serviceDisabled):
+                return .unlocked
+            
+            default:
+                return fromState
             }
         }
         
-        guard !Task.isCancelled else { return }
+        stateMachine.addAnyHandler(.any => .any) { [weak self] context in
+            guard let self, context.fromState != context.toState else { return }
+            
+            MXLog.info("Transitioning from `\(context.fromState)` to `\(context.toState)` with event `\(String(describing: context.event))`.")
+            
+            switch (context.fromState, context.toState) {
+            case (_, .appObscured):
+                showPlaceholder()
+            case (_, .backgrounded):
+                appLockService.applicationDidEnterBackground()
+                showPlaceholder() // Double call but just to be safe.
+            case (_, .attemptingBiometricUnlock):
+                showPlaceholder() // For the unlock background. Triple call but just to be safe.
+                Task { await self.attemptBiometricUnlock() }
+            case (.attemptingBiometricUnlock, .dismissingBiometricUnlock):
+                break // Transitional state, no need to do anything.
+            case (_, .attemptingPINUnlock):
+                showUnlockScreen()
+            case (_, .unlocked):
+                actionsSubject.send(.unlockApp)
+            case (_, .loggingOut):
+                actionsSubject.send(.forceLogout)
+            default:
+                fatalError("Unhandled transition.")
+            }
+        }
         
-        showUnlockScreen()
+        stateMachine.addErrorHandler { context in
+            fatalError("Unexpected transition from `\(context.fromState)` to `\(context.toState)` with event `\(String(describing: context.event))`.")
+        }
     }
     
+    // MARK: - App unlock
+    
     /// Displays the unlock flow with the app's placeholder view to hide obscure the view hierarchy in the app switcher.
     private func showPlaceholder() {
         navigationCoordinator.setRootCoordinator(PlaceholderScreenCoordinator(showsBackgroundGradient: true), animated: false)
         actionsSubject.send(.lockApp)
     }
     
+    /// Attempts to authenticate the user using Face ID, Touch ID or (possibly) Optic ID.
+    private func attemptBiometricUnlock() async {
+        let result = await appLockService.unlockWithBiometrics()
+        stateMachine.tryEvent(.didFinishBiometricUnlock(result))
+    }
+    
     /// Displays the unlock flow with the main unlock screen.
     private func showUnlockScreen() {
         let coordinator = AppLockScreenCoordinator(parameters: .init(appLockService: appLockService))
@@ -132,10 +226,9 @@ class AppLockFlowCoordinator: CoordinatorProtocol {
             guard let self else { return }
             switch action {
             case .appUnlocked:
-                guard UIApplication.shared.applicationState == .active else { return }
+                stateMachine.tryEvent(.didUnlockWithPIN)
-                actionsSubject.send(.unlockApp)
             case .forceLogout:
-                actionsSubject.send(.forceLogout)
+                stateMachine.tryEvent(.forceLogout)
             }
         }
         .store(in: &cancellables)

ElementX/Sources/Mocks/Generated/GeneratedMocks.swift

@@ -128,6 +128,11 @@ class AppLockServiceMock: AppLockServiceProtocol {
         set(value) { underlyingIsEnabled = value }
     }
     var underlyingIsEnabled: Bool!
+    var isEnabledPublisher: AnyPublisher<Bool, Never> {
+        get { return underlyingIsEnabledPublisher }
+        set(value) { underlyingIsEnabledPublisher = value }
+    }
+    var underlyingIsEnabledPublisher: AnyPublisher<Bool, Never>!
     var biometryType: LABiometryType {
         get { return underlyingBiometryType }
         set(value) { underlyingBiometryType = value }
@@ -292,10 +297,10 @@ class AppLockServiceMock: AppLockServiceProtocol {
     var unlockWithBiometricsCalled: Bool {
         return unlockWithBiometricsCallsCount > 0
     }
-    var unlockWithBiometricsReturnValue: Bool!
+    var unlockWithBiometricsReturnValue: AppLockServiceBiometricResult!
-    var unlockWithBiometricsClosure: (() async -> Bool)?
+    var unlockWithBiometricsClosure: (() async -> AppLockServiceBiometricResult)?
 
-    func unlockWithBiometrics() async -> Bool {
+    func unlockWithBiometrics() async -> AppLockServiceBiometricResult {
         unlockWithBiometricsCallsCount += 1
         if let unlockWithBiometricsClosure = unlockWithBiometricsClosure {
             return await unlockWithBiometricsClosure()

ElementX/Sources/Screens/AppLock/AppLockSetupBiometricsScreen/AppLockSetupBiometricsScreenViewModel.swift

@@ -56,7 +56,7 @@ class AppLockSetupBiometricsScreenViewModel: AppLockSetupBiometricsScreenViewMod
         
         // Attempt unlock to trigger Face ID permissions alert.
         if appLockService.biometryType == .faceID,
-           await !appLockService.unlockWithBiometrics() {
+           await appLockService.unlockWithBiometrics() != .unlocked {
             MXLog.info("Confirmation failed. Disabling biometric unlock.")
             appLockService.disableBiometricUnlock()
         }

ElementX/Sources/Screens/AppLock/AppLockSetupSettingsScreen/AppLockSetupSettingsScreenViewModel.swift

@@ -62,7 +62,7 @@ class AppLockSetupSettingsScreenViewModel: AppLockSetupSettingsScreenViewModelTy
             
             // Attempt unlock to trigger Face ID permissions alert.
             if appLockService.biometryType == .faceID,
-               await !appLockService.unlockWithBiometrics() {
+               await appLockService.unlockWithBiometrics() != .unlocked {
                 MXLog.info("Confirmation failed. Disabling biometric unlock.")
                 state.bindings.enableBiometrics = false
                 appLockService.disableBiometricUnlock()

ElementX/Sources/Services/AppLock/AppLockService.swift

@@ -38,6 +38,9 @@ class AppLockService: AppLockServiceProtocol {
         }
     }
     
+    private var isEnabledSubject: PassthroughSubject<Bool, Never> = .init()
+    var isEnabledPublisher: AnyPublisher<Bool, Never> { isEnabledSubject.eraseToAnyPublisher() }
+    
     var biometryType: LABiometryType {
         updateBiometrics()
         guard context.evaluatedPolicyDomainState != nil else { return .none }
@@ -71,6 +74,7 @@ class AppLockService: AppLockServiceProtocol {
         
         do {
             try keychainController.setPINCode(pinCode)
+            isEnabledSubject.send(true)
             return .success(())
         } catch {
             MXLog.error("Keychain access error: \(error)")
@@ -105,6 +109,7 @@ class AppLockService: AppLockServiceProtocol {
         keychainController.removePINCode()
         keychainController.removePINCodeBiometricState()
         appSettings.appLockNumberOfPINAttempts = 0
+        isEnabledSubject.send(false)
     }
     
     func applicationDidEnterBackground() {
@@ -128,30 +133,35 @@ class AppLockService: AppLockServiceProtocol {
             _ = enableBiometricUnlock()
         }
         
-        return completeUnlock()
+        completeUnlock()
+        return true
     }
     
-    func unlockWithBiometrics() async -> Bool {
+    func unlockWithBiometrics() async -> AppLockServiceBiometricResult {
         guard biometryType != .none, biometricUnlockEnabled else {
             MXLog.error("Biometric unlock not setup.")
-            return false
+            return .failed
         }
         
         guard biometricUnlockTrusted else {
             MXLog.error("Biometrics have changed. PIN should be shown.")
-            return false
+            return .failed
         }
         
         do {
             let context = unlockContext()
             guard try await context.evaluatePolicy(unlockPolicy, localizedReason: L10n.screenAppLockBiometricUnlockReasonIos) else {
                 MXLog.warning("\(context.biometryType) failed without error.")
-                return false
+                return .failed
             }
-            return completeUnlock()
+            completeUnlock()
+            return .unlocked
+        } catch LAError.systemCancel {
+            MXLog.error("\(context.biometryType) failed: The system cancelled.")
+            return .interrupted
         } catch {
             MXLog.error("\(context.biometryType) failed: \(error)")
-            return false
+            return .failed
         }
     }
     
@@ -180,9 +190,8 @@ class AppLockService: AppLockServiceProtocol {
     }
     
     /// Shared logic for completing an unlock via a PIN or biometrics.
-    private func completeUnlock() -> Bool {
+    private func completeUnlock() {
         timer.registerUnlock()
         appSettings.appLockNumberOfPINAttempts = 0
-        return true
     }
 }

ElementX/Sources/Services/AppLock/AppLockServiceProtocol.swift

@@ -30,6 +30,18 @@ enum AppLockServiceError: Error {
     case biometricUnlockNotSupported
 }
 
+/// The result of an attempt to unlock the app using Touch ID or Face ID.
+enum AppLockServiceBiometricResult {
+    /// Biometric lock was successful.
+    case unlocked
+    /// Biometric lock failed to authenticate the user. This represents any failure
+    /// other than the app being backgrounded during the authentication.
+    case failed
+    /// Biometric lock was interrupted by the system and did not complete. The expected
+    /// cause for this that the app was backgrounded whilst the request was in progress.
+    case interrupted
+}
+
 @MainActor
 protocol AppLockServiceProtocol: AnyObject {
     /// The use of a PIN code is mandatory for this device.
@@ -37,6 +49,9 @@ protocol AppLockServiceProtocol: AnyObject {
     /// The app has been configured to automatically lock with a PIN code.
     var isEnabled: Bool { get }
     
+    /// A publisher that advertises when the service has been enabled or disabled.
+    var isEnabledPublisher: AnyPublisher<Bool, Never> { get }
+    
     /// The type of biometric authentication supported by the device.
     var biometryType: LABiometryType { get }
     /// Whether or not the user has enabled unlock via TouchID, FaceID or (possibly) OpticID.
@@ -64,7 +79,7 @@ protocol AppLockServiceProtocol: AnyObject {
     /// Attempt to unlock the app with the supplied PIN code.
     func unlock(with pinCode: String) -> Bool
     /// Attempt to unlock the app using FaceID or TouchID.
-    func unlockWithBiometrics() async -> Bool
+    func unlockWithBiometrics() async -> AppLockServiceBiometricResult
     
     /// The number of attempts the user had made to unlock with a PIN code.
     ///

ElementX/Sources/UITests/UITestsAppCoordinator.swift

@@ -201,7 +201,8 @@ class MockScreen: Identifiable {
                 fatalError("Failed to start listening for notifications.")
             }
             
-            let coordinator = AppLockFlowCoordinator(appLockService: appLockService,
+            let coordinator = AppLockFlowCoordinator(initialState: .unlocked,
+                                                     appLockService: appLockService,
                                                      navigationCoordinator: navigationCoordinator,
                                                      notificationCenter: notificationCenter)
             

UnitTests/Sources/AppLock/AppLockServiceTests.swift

@@ -207,7 +207,7 @@ class AppLockServiceTests: XCTestCase {
         XCTAssertEqual(service.biometryType, .touchID, "The biometry type should not change.")
         XCTAssertTrue(service.biometricUnlockEnabled, "Biometric unlock should now be enabled.")
         XCTAssertTrue(service.biometricUnlockTrusted, "Biometric unlock should now be trusted.")
-        guard await service.unlockWithBiometrics() else {
+        guard await service.unlockWithBiometrics() == .unlocked else {
             XCTFail("The biometric unlock should work.")
             return
         }

changelog.d/2134.bugfix

@@ -0,0 +1 @@
+Fix a bug where cancelling Face ID unlock would loop back round and present Face ID again.