Restore the .oidcCallback route for external authentication. (#5391)

* Restore the .oidcCallback route (partially reverts #3461) for external authentication. * Make sure OIDC also works for non-http URLs. * Remove oidcAuthentication from the state machine. There isn't a reliable way to detect failure/cancellation when e.g. the user returns from an external app without interacting with the MAS page.
2026-11-03 16:10:20 +00:00 · 2026-05-05 12:47:07 +01:00
parent aaf81e00f3
commit fef942f529
17 changed files with 181 additions and 52 deletions
@@ -40,8 +40,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
        case serverConfirmationScreen
        /// The screen to choose a different server.
        case serverSelectionScreen
-        /// The web authentication session is being presented.
-        case oidcAuthentication
        /// The screen to login with a password.
        case loginScreen
        
@@ -76,10 +74,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
        /// The user is no longer selecting a server.
        case dismissedServerSelection
        
-        /// Show the web authentication session for OIDC (using the parameters in the `userInfo`).
-        case continueWithOIDC
-        /// The web authentication session was aborted.
-        case cancelledOIDCAuthentication(previousState: State)
        /// Show the screen to login with password (with the optional login hint in the `userInfo`).
        case continueWithPassword
        /// The password login was aborted.
@@ -157,6 +151,8 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
    }
    
    func clearRoute(animated: Bool) {
+        oidcPresenter?.cancel() // Handle ongoing OIDC authentication first.
+        
        switch stateMachine.state {
        case .initial, .startScreen:
            break
@@ -168,9 +164,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
        case .serverSelectionScreen:
            navigationStackCoordinator.setSheetCoordinator(nil)
            navigationStackCoordinator.popToRoot(animated: animated)
-        case .oidcAuthentication:
-            oidcPresenter?.cancel()
-            navigationStackCoordinator.popToRoot(animated: animated)
        case .loginScreen:
            navigationStackCoordinator.popToRoot(animated: animated)
        case .bugReportFlow:
@@ -182,6 +175,15 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
        }
    }
    
+    func handleOIDCRedirectURL(_ url: URL) {
+        guard let oidcPresenter else {
+            MXLog.error("Failed to find an OIDC request in progress.")
+            return
+        }
+        
+        oidcPresenter.handleUniversalLinkCallback(url)
+    }
+    
    // MARK: - Setup
    
    private func configureStateMachine() {
@@ -220,16 +222,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
        }
        stateMachine.addRoutes(event: .dismissedServerSelection, transitions: [.serverSelectionScreen => .serverConfirmationScreen])
        
-        stateMachine.addRoutes(event: .continueWithOIDC, transitions: [.serverConfirmationScreen => .oidcAuthentication,
-                                                                       .startScreen => .oidcAuthentication]) { [weak self] context in
-            guard let (oidcData, window) = context.userInfo as? (OIDCAuthorizationDataProxy, UIWindow) else {
-                fatalError("Missing the OIDC data and presentation anchor.")
-            }
-            self?.showOIDCAuthentication(oidcData: oidcData, presentationAnchor: window, fromState: context.fromState)
-        }
-        stateMachine.addRoutes(event: .cancelledOIDCAuthentication(previousState: .serverConfirmationScreen), transitions: [.oidcAuthentication => .serverConfirmationScreen])
-        stateMachine.addRoutes(event: .cancelledOIDCAuthentication(previousState: .startScreen), transitions: [.oidcAuthentication => .startScreen])
-        
        stateMachine.addRoutes(event: .continueWithPassword, transitions: [.serverConfirmationScreen => .loginScreen,
                                                                           .startScreen => .loginScreen]) { [weak self] context in
            let loginHint = context.userInfo as? String
@@ -255,7 +247,8 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
        // Completion
        
        stateMachine.addRoutes(event: .signedIn, transitions: [.qrCodeLoginScreen => .complete,
-                                                               .oidcAuthentication => .complete,
+                                                               .serverConfirmationScreen => .complete, // OIDC authentication
+                                                               .startScreen => .complete, // Direct OIDC authentication
                                                               .loginScreen => .complete]) { [weak self] context in
            guard let userSession = context.userInfo as? UserSessionProtocol else { fatalError("The user session wasn't included in the context") }
            self?.userHasSignedIn(userSession: userSession)
@@ -308,7 +301,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
                    stateMachine.tryEvent(.confirmServer(.register))
                    
                case .loginDirectlyWithOIDC(let oidcData, let window):
-                    stateMachine.tryEvent(.continueWithOIDC, userInfo: (oidcData, window))
+                    showOIDCAuthentication(oidcData: oidcData, presentationAnchor: window)
                case .loginDirectlyWithPassword(let loginHint):
                    stateMachine.tryEvent(.continueWithPassword, userInfo: loginHint)
                
@@ -382,7 +375,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
            
            switch action {
            case .continueWithOIDC(let oidcData, let window):
-                stateMachine.tryEvent(.continueWithOIDC, userInfo: (oidcData, window))
+                showOIDCAuthentication(oidcData: oidcData, presentationAnchor: window)
            case .continueWithPassword:
                stateMachine.tryEvent(.continueWithPassword)
            case .changeServer:
@@ -424,10 +417,14 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
        }
    }
    
-    private func showOIDCAuthentication(oidcData: OIDCAuthorizationDataProxy, presentationAnchor: UIWindow, fromState: State) {
+    /// **Note:** We have intentionally excluded this presentation from the state machine as it doesn't mutate our navigation stack and there
+    /// isn't a robust way to detect why the user returned to the app when the MAS URL directly opens an external app for authentication without
+    /// presenting a web authentication session.
+    private func showOIDCAuthentication(oidcData: OIDCAuthorizationDataProxy, presentationAnchor: UIWindow) {
        let presenter = OIDCAuthenticationPresenter(authenticationService: authenticationService,
                                                    oidcRedirectURL: appSettings.oidcRedirectURL,
                                                    presentationAnchor: presentationAnchor,
+                                                    appMediator: appMediator,
                                                    userIndicatorController: userIndicatorController)
        oidcPresenter = presenter
        
@@ -436,8 +433,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
            case .success(let userSession):
                stateMachine.tryEvent(.signedIn, userInfo: userSession)
            case .failure:
-                stateMachine.tryEvent(.cancelledOIDCAuthentication(previousState: fromState))
-                // Nothing more to do, the alerts are handled by the presenter.
+                break // Nothing to do, any alerts will be handled by the presenter.
            }
            oidcPresenter = nil
        }
@@ -174,7 +174,7 @@ class ChatsTabFlowCoordinator: FlowCoordinatorProtocol {
            }
        case .globalSearch:
            presentGlobalSearch()
-        case .accountProvisioningLink, .settings, .chatBackupSettings, .call:
+        case .accountProvisioningLink, .oidcCallback, .settings, .chatBackupSettings, .call:
            break // These routes cannot be handled.
        }
    }
@@ -690,6 +690,7 @@ class ChatsTabFlowCoordinator: FlowCoordinatorProtocol {
    private func startEncryptionResetFlow(animated: Bool) {
        let sheetNavigationStackCoordinator = NavigationStackCoordinator()
        let parameters = EncryptionResetFlowCoordinatorParameters(userSession: userSession,
+                                                                  appMediator: flowParameters.appMediator,
                                                                  appSettings: flowParameters.appSettings,
                                                                  userIndicatorController: flowParameters.userIndicatorController,
                                                                  navigationStackCoordinator: sheetNavigationStackCoordinator,
@@ -19,6 +19,7 @@ enum EncryptionResetFlowCoordinatorAction: Equatable {

 struct EncryptionResetFlowCoordinatorParameters {
    let userSession: UserSessionProtocol
+    let appMediator: AppMediatorProtocol
    let appSettings: AppSettings
    let userIndicatorController: UserIndicatorControllerProtocol
    let navigationStackCoordinator: NavigationStackCoordinator
@@ -27,6 +28,7 @@ struct EncryptionResetFlowCoordinatorParameters {

 class EncryptionResetFlowCoordinator: FlowCoordinatorProtocol {
    private let userSession: UserSessionProtocol
+    private let appMediator: AppMediatorProtocol
    private let appSettings: AppSettings
    private let userIndicatorController: UserIndicatorControllerProtocol
    
@@ -62,6 +64,7 @@ class EncryptionResetFlowCoordinator: FlowCoordinatorProtocol {
    
    init(parameters: EncryptionResetFlowCoordinatorParameters) {
        userSession = parameters.userSession
+        appMediator = parameters.appMediator
        appSettings = parameters.appSettings
        userIndicatorController = parameters.userIndicatorController
        navigationStackCoordinator = parameters.navigationStackCoordinator
@@ -158,6 +161,7 @@ class EncryptionResetFlowCoordinator: FlowCoordinatorProtocol {
        // As of iOS 16, there is an issue on the simulator with accessing the cookie but it works on a device. 🤷‍♂️
        accountSettingsPresenter = OIDCAccountSettingsPresenter(accountURL: url,
                                                                presentationAnchor: windowManager.mainWindow,
+                                                                appMediator: appMediator,
                                                                appSettings: appSettings)
        accountSettingsPresenter?.start()
    }
@@ -80,8 +80,8 @@ class EncryptionSettingsFlowCoordinator: FlowCoordinatorProtocol {
        MXLog.info("Handling app route: \(appRoute)")
        
        switch appRoute {
-        case .accountProvisioningLink:
-            break // We always ignore this flow when logged in.
+        case .accountProvisioningLink, .oidcCallback:
+            break // We always ignore these flows when logged in.
        case .roomList, .room, .roomAlias, .childRoom, .childRoomAlias,
             .roomDetails, .roomMemberDetails, .userProfile, .thread,
             .event, .eventOnRoomAlias, .childEvent, .childEventOnRoomAlias,
@@ -20,6 +20,7 @@ class OnboardingFlowCoordinator: FlowCoordinatorProtocol {
    private let userSession: UserSessionProtocol
    private let appLockService: AppLockServiceProtocol
    private let analyticsService: AnalyticsService
+    private let appMediator: AppMediatorProtocol
    private let appSettings: AppSettings
    private let notificationManager: NotificationManagerProtocol
    private let userIndicatorController: UserIndicatorControllerProtocol
@@ -66,6 +67,7 @@ class OnboardingFlowCoordinator: FlowCoordinatorProtocol {
        userSession = flowParameters.userSession
        self.appLockService = appLockService
        analyticsService = flowParameters.analytics
+        appMediator = flowParameters.appMediator
        appSettings = flowParameters.appSettings
        notificationManager = flowParameters.notificationManager
        userIndicatorController = flowParameters.userIndicatorController
@@ -315,6 +317,7 @@ class OnboardingFlowCoordinator: FlowCoordinatorProtocol {
    private func startEncryptionResetFlow() {
        let resetNavigationStackCoordinator = NavigationStackCoordinator()
        let coordinator = EncryptionResetFlowCoordinator(parameters: .init(userSession: userSession,
+                                                                           appMediator: appMediator,
                                                                           appSettings: appSettings,
                                                                           userIndicatorController: userIndicatorController,
                                                                           navigationStackCoordinator: resetNavigationStackCoordinator,
@@ -201,7 +201,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
            }
        case .roomAlias, .childRoomAlias, .eventOnRoomAlias, .childEventOnRoomAlias:
            break // These are converted to a room ID route one level above.
-        case .accountProvisioningLink, .roomList, .userProfile, .call, .settings, .chatBackupSettings, .globalSearch:
+        case .accountProvisioningLink, .oidcCallback, .roomList, .userProfile, .call, .settings, .chatBackupSettings, .globalSearch:
            break // These routes can't be handled.
        case .transferOwnership(let roomID):
            guard self.roomID == roomID else { fatalError("Navigation route doesn't belong to this room flow.") }
@@ -120,7 +120,8 @@ final class RoomMembersFlowCoordinator: FlowCoordinatorProtocol {
            }
        case .roomAlias, .childRoomAlias, .eventOnRoomAlias, .childEventOnRoomAlias:
            break // These are converted to a room ID route one level above.
-        case .accountProvisioningLink, .roomList, .room, .roomDetails, .event,
+        case .accountProvisioningLink, .oidcCallback,
+             .roomList, .room, .roomDetails, .event,
             .userProfile, .call, .settings, .chatBackupSettings,
             .share, .transferOwnership, .thread, .globalSearch:
            break
@@ -303,6 +303,7 @@ class SettingsFlowCoordinator: FlowCoordinatorProtocol {
        // As of iOS 16, there is an issue on the simulator with accessing the cookie but it works on a device. 🤷‍♂️
        accountSettingsPresenter = OIDCAccountSettingsPresenter(accountURL: url,
                                                                presentationAnchor: flowParameters.windowManager.mainWindow,
+                                                                appMediator: flowParameters.appMediator,
                                                                appSettings: flowParameters.appSettings,
                                                                continuation: continuation)
        accountSettingsPresenter?.start()
@@ -121,8 +121,8 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
        MXLog.info("Handling app route: \(appRoute)")
        
        switch appRoute {
-        case .accountProvisioningLink:
-            break // We always ignore this flow when logged in.
+        case .accountProvisioningLink, .oidcCallback:
+            break // We always ignore these flows when logged in.
        case .settings, .chatBackupSettings:
            if ProcessInfo.processInfo.isiOSAppOnMac, flowParameters.windowManager.secondaryWindowsEnabled {
                startSettingsFlow(detached: true)