From cda3a62a7a2091e09d068f32d2b5ca051409cf24 Mon Sep 17 00:00:00 2001 From: Doug Date: Mon, 30 Oct 2023 11:58:21 +0000 Subject: [PATCH 1/3] Enforce mandatory app lock outside of the authentication flow. --- .../Sources/Application/AppCoordinator.swift | 65 +++++++++++++++---- .../AppCoordinatorStateMachine.swift | 16 +++++ .../AppLockSetupFlowCoordinator.swift | 6 +- 3 files changed, 75 insertions(+), 12 deletions(-) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index b76fa3a321..7b5d145437 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -45,6 +45,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate, private var authenticationCoordinator: AuthenticationCoordinator? private let appLockFlowCoordinator: AppLockFlowCoordinator + private var appLockSetupFlowCoordinator: AppLockSetupFlowCoordinator? private var userSessionFlowCoordinator: UserSessionFlowCoordinator? private var softLogoutCoordinator: SoftLogoutScreenCoordinator? @@ -143,7 +144,18 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate, return } - stateMachine.processEvent(userSessionStore.hasSessions ? .startWithExistingSession : .startWithAuthentication) + guard userSessionStore.hasSessions else { + stateMachine.processEvent(.startWithAuthentication) + return + } + + if appSettings.appLockFlowEnabled, + appSettings.appLockIsMandatory, + !appLockFlowCoordinator.appLockService.isEnabled { + stateMachine.processEvent(.startWithAppLockSetup) + } else { + stateMachine.processEvent(.startWithExistingSession) + } } func stop() { @@ -319,28 +331,34 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate, switch (context.fromState, context.event, context.toState) { case (.initial, .startWithAuthentication, .signedOut): - self.startAuthentication() + startAuthentication() case (.signedOut, .createdUserSession, .signedIn): - self.setupUserSession() + setupUserSession() case (.initial, .startWithExistingSession, .restoringSession): - self.restoreUserSession() + restoreUserSession() case (.restoringSession, .failedRestoringSession, .signedOut): - self.showLoginErrorToast() - self.presentSplashScreen() + showLoginErrorToast() + presentSplashScreen() case (.restoringSession, .createdUserSession, .signedIn): - self.setupUserSession() + setupUserSession() + + case (.initial, .startWithAppLockSetup, .mandatoryAppLockSetup): + startMandatoryAppLockSetup() + case (.mandatoryAppLockSetup, .appLockSetupComplete, .restoringSession): + restoreUserSession() + case (.signingOut, .signOut, .signingOut): // We can ignore signOut when already in the process of signing out, // such as the SDK sending an authError due to token invalidation. break case (_, .signOut(let isSoft, _), .signingOut): - self.logout(isSoft: isSoft) + logout(isSoft: isSoft) case (.signingOut(_, let disableAppLock), .completedSigningOut, .signedOut): - self.presentSplashScreen(isSoftLogout: false, disableAppLock: disableAppLock) + presentSplashScreen(isSoftLogout: false, disableAppLock: disableAppLock) case (.signingOut(_, let disableAppLock), .showSoftLogout, .softLogout): - self.presentSplashScreen(isSoftLogout: true, disableAppLock: disableAppLock) + presentSplashScreen(isSoftLogout: true, disableAppLock: disableAppLock) case (.signedIn, .clearCache, .initial): - self.clearCache() + clearCache() default: fatalError("Unknown transition: \(context)") } @@ -464,6 +482,31 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate, } } + /// Used to add a PIN code to an existing session that somehow missed out mandatory PIN setup. + private func startMandatoryAppLockSetup() { + MXLog.info("Mandatory App Lock enabled but no PIN is set. Showing the setup flow.") + + let navigationCoordinator = NavigationStackCoordinator() + let coordinator = AppLockSetupFlowCoordinator(presentingFlow: .authentication, + appLockService: appLockFlowCoordinator.appLockService, + navigationStackCoordinator: navigationCoordinator) + coordinator.actions.sink { [weak self] action in + guard let self else { return } + switch action { + case .complete: + stateMachine.processEvent(.appLockSetupComplete) + appLockSetupFlowCoordinator = nil + case .forceLogout: + fatalError("Creating a PIN shouldn't be able to fail in this way") + } + } + .store(in: &cancellables) + + appLockSetupFlowCoordinator = coordinator + navigationRootCoordinator.setRootCoordinator(navigationCoordinator) + coordinator.start() + } + private func logout(isSoft: Bool) { guard let userSession else { fatalError("User session not setup") diff --git a/ElementX/Sources/Application/AppCoordinatorStateMachine.swift b/ElementX/Sources/Application/AppCoordinatorStateMachine.swift index ea05f9d33b..47052ef27a 100644 --- a/ElementX/Sources/Application/AppCoordinatorStateMachine.swift +++ b/ElementX/Sources/Application/AppCoordinatorStateMachine.swift @@ -29,6 +29,11 @@ class AppCoordinatorStateMachine { /// Opening an existing session. case restoringSession + /// Showing the mandatory app lock setup flow before restoring the session. + /// This state should only be allowed before restoring an existing session. For + /// new users the setup is inserted in the middle of the authentication flow. + case mandatoryAppLockSetup + /// User session started case signedIn @@ -44,12 +49,20 @@ class AppCoordinatorStateMachine { /// Start the `AppCoordinator` by restoring an existing account. case startWithExistingSession + /// Start the `AppCoordinator` by showing the mandatory PIN creation flow. + /// This event should only be sent if an account exists and a mandatory PIN is + /// missing. Normally it will be handled as part of the authentication flow. + case startWithAppLockSetup + /// Restoring session failed. case failedRestoringSession /// A session has been created. case createdUserSession + /// The app lock setup has been completed. + case appLockSetupComplete + /// Request sign out. case signOut(isSoft: Bool, disableAppLock: Bool) /// Request the soft logout screen. @@ -80,6 +93,9 @@ class AppCoordinatorStateMachine { stateMachine.addRoutes(event: .createdUserSession, transitions: [.restoringSession => .signedIn]) stateMachine.addRoutes(event: .failedRestoringSession, transitions: [.restoringSession => .signedOut]) + stateMachine.addRoutes(event: .startWithAppLockSetup, transitions: [.initial => .mandatoryAppLockSetup]) + stateMachine.addRoutes(event: .appLockSetupComplete, transitions: [.mandatoryAppLockSetup => .restoringSession]) + stateMachine.addRoutes(event: .completedSigningOut, transitions: [.signingOut(isSoft: false, disableAppLock: false) => .signedOut, .signingOut(isSoft: false, disableAppLock: true) => .signedOut]) stateMachine.addRoutes(event: .showSoftLogout, transitions: [.signingOut(isSoft: true, disableAppLock: false) => .softLogout]) diff --git a/ElementX/Sources/FlowCoordinators/AppLockSetupFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/AppLockSetupFlowCoordinator.swift index 11a46a97d4..6b80a0fe55 100644 --- a/ElementX/Sources/FlowCoordinators/AppLockSetupFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/AppLockSetupFlowCoordinator.swift @@ -201,7 +201,11 @@ class AppLockSetupFlowCoordinator: FlowCoordinatorProtocol { .store(in: &cancellables) if presentingFlow == .authentication { - navigationStackCoordinator.push(coordinator) + if navigationStackCoordinator.rootCoordinator == nil { + navigationStackCoordinator.setRootCoordinator(coordinator) + } else { + navigationStackCoordinator.push(coordinator) + } } else { modalNavigationStackCoordinator.setRootCoordinator(coordinator) navigationStackCoordinator.setSheetCoordinator(modalNavigationStackCoordinator) From 8871bd18320ba32c41ce678d295a7b5095569ebb Mon Sep 17 00:00:00 2001 From: Doug Date: Mon, 30 Oct 2023 12:02:59 +0000 Subject: [PATCH 2/3] Rename case to make more sense. --- ElementX/Sources/Application/AppCoordinator.swift | 2 +- .../AppLockSetupFlowCoordinator.swift | 14 +++++++------- .../Authentication/AuthenticationCoordinator.swift | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index 7b5d145437..ba8e39a7bc 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -487,7 +487,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationCoordinatorDelegate, MXLog.info("Mandatory App Lock enabled but no PIN is set. Showing the setup flow.") let navigationCoordinator = NavigationStackCoordinator() - let coordinator = AppLockSetupFlowCoordinator(presentingFlow: .authentication, + let coordinator = AppLockSetupFlowCoordinator(presentingFlow: .onboarding, appLockService: appLockFlowCoordinator.appLockService, navigationStackCoordinator: navigationCoordinator) coordinator.actions.sink { [weak self] action in diff --git a/ElementX/Sources/FlowCoordinators/AppLockSetupFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/AppLockSetupFlowCoordinator.swift index 6b80a0fe55..70f517a588 100644 --- a/ElementX/Sources/FlowCoordinators/AppLockSetupFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/AppLockSetupFlowCoordinator.swift @@ -34,8 +34,8 @@ class AppLockSetupFlowCoordinator: FlowCoordinatorProtocol { /// The presentation context of the flow. enum PresentationFlow { - /// The flow is shown as for mandatory PIN creation in the authentication flow - case authentication + /// The flow is shown for mandatory PIN creation in the authentication flow or on app launch. + case onboarding /// The flow is shown from the Settings screen. case settings } @@ -113,7 +113,7 @@ class AppLockSetupFlowCoordinator: FlowCoordinatorProtocol { switch (event, fromState) { case (.start, .initial): - if presentingFlow == .authentication { return .createPIN(replacingExitingPIN: false) } + if presentingFlow == .onboarding { return .createPIN(replacingExitingPIN: false) } return appLockService.isEnabled ? .unlock : .createPIN(replacingExitingPIN: false) case (.pinEntered, .unlock): return .settings @@ -122,7 +122,7 @@ class AppLockSetupFlowCoordinator: FlowCoordinatorProtocol { case (.forceLogout, .unlock): return .loggingOut case (.pinEntered, .createPIN(let replacingExitingPIN)): - if presentingFlow == .authentication { + if presentingFlow == .onboarding { return appLockService.biometryType != .none ? .biometricsPrompt : .complete } else if !replacingExitingPIN { return appLockService.biometricUnlockEnabled || appLockService.biometryType == .none ? .settings : .biometricsPrompt @@ -182,7 +182,7 @@ class AppLockSetupFlowCoordinator: FlowCoordinatorProtocol { private func showCreatePIN() { // Despite appLockService.isMandatory existing, we don't use that here, // to allow for cancellation when changing the PIN code within settings. - let isMandatory = presentingFlow == .authentication + let isMandatory = presentingFlow == .onboarding let coordinator = AppLockSetupPINScreenCoordinator(parameters: .init(initialMode: .create, isMandatory: isMandatory, @@ -200,7 +200,7 @@ class AppLockSetupFlowCoordinator: FlowCoordinatorProtocol { } .store(in: &cancellables) - if presentingFlow == .authentication { + if presentingFlow == .onboarding { if navigationStackCoordinator.rootCoordinator == nil { navigationStackCoordinator.setRootCoordinator(coordinator) } else { @@ -223,7 +223,7 @@ class AppLockSetupFlowCoordinator: FlowCoordinatorProtocol { } .store(in: &cancellables) - if presentingFlow == .authentication { + if presentingFlow == .onboarding { navigationStackCoordinator.push(coordinator) } else { modalNavigationStackCoordinator.push(coordinator) diff --git a/ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift b/ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift index af5736ec62..c218354fe1 100644 --- a/ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift +++ b/ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift @@ -251,7 +251,7 @@ class AuthenticationCoordinator: CoordinatorProtocol { } private func showAppLockSetupFlow(userSession: UserSessionProtocol) { - let coordinator = AppLockSetupFlowCoordinator(presentingFlow: .authentication, + let coordinator = AppLockSetupFlowCoordinator(presentingFlow: .onboarding, appLockService: appLockService, navigationStackCoordinator: navigationStackCoordinator) coordinator.actions.sink { [weak self] action in From dfc50231270b66b4bea99cb3234ad51671872938 Mon Sep 17 00:00:00 2001 From: Doug Date: Mon, 30 Oct 2023 12:06:40 +0000 Subject: [PATCH 3/3] Changelog. --- changelog.d/pr-1982.wip | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/pr-1982.wip diff --git a/changelog.d/pr-1982.wip b/changelog.d/pr-1982.wip new file mode 100644 index 0000000000..9d5f279823 --- /dev/null +++ b/changelog.d/pr-1982.wip @@ -0,0 +1 @@ +Enforce mandatory app lock outside of the authentication flow. \ No newline at end of file