Update OIDC presentation (#987)

* Present OIDC directly on the ServerConfirmationScreen.

Read the actual window from the view.

* Pop back to the confirmation screen when a user enters an MXID that needs OIDC.

* Remove OIDC error handling from the login screen.

ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift

@@ -26,15 +26,18 @@ protocol AuthenticationCoordinatorDelegate: AnyObject {
 class AuthenticationCoordinator: CoordinatorProtocol {
     private let authenticationService: AuthenticationServiceProxyProtocol
     private let navigationStackCoordinator: NavigationStackCoordinator
+    private let serviceLocator: ServiceLocator
     
     private var cancellables: Set<AnyCancellable> = []
     
     weak var delegate: AuthenticationCoordinatorDelegate?
     
     init(authenticationService: AuthenticationServiceProxyProtocol,
-         navigationStackCoordinator: NavigationStackCoordinator) {
+         navigationStackCoordinator: NavigationStackCoordinator,
+         serviceLocator: ServiceLocator = .shared) {
         self.authenticationService = authenticationService
         self.navigationStackCoordinator = navigationStackCoordinator
+        self.serviceLocator = serviceLocator
     }
     
     func start() {
@@ -64,7 +67,7 @@ class AuthenticationCoordinator: CoordinatorProtocol {
     private func startAuthentication() async {
         startLoading()
         
-        switch await authenticationService.configure(for: ServiceLocator.shared.settings.defaultHomeserverAddress) {
+        switch await authenticationService.configure(for: serviceLocator.settings.defaultHomeserverAddress) {
         case .success:
             stopLoading()
             showServerConfirmationScreen()
@@ -76,7 +79,7 @@ class AuthenticationCoordinator: CoordinatorProtocol {
     
     private func showServerSelectionScreen(isModallyPresented: Bool) {
         let navigationCoordinator = NavigationStackCoordinator()
-        let userIndicatorController: UserIndicatorControllerProtocol! = isModallyPresented ? UserIndicatorController(rootCoordinator: navigationCoordinator) : ServiceLocator.shared.userIndicatorController
+        let userIndicatorController: UserIndicatorControllerProtocol! = isModallyPresented ? UserIndicatorController(rootCoordinator: navigationCoordinator) : serviceLocator.userIndicatorController
         
         let parameters = ServerSelectionScreenCoordinatorParameters(authenticationService: authenticationService,
                                                                     userIndicatorController: userIndicatorController,
@@ -115,10 +118,14 @@ class AuthenticationCoordinator: CoordinatorProtocol {
             guard let self else { return }
             
             switch action {
-            case .confirm:
-                self.showLoginScreen()
+            case .continue(let window):
+                if authenticationService.homeserver.value.loginMode == .oidc, let window {
+                    showOIDCAuthentication(presentationAnchor: window)
+                } else {
+                    showLoginScreen()
+                }
             case .changeServer:
-                self.showServerSelectionScreen(isModallyPresented: true)
+                showServerSelectionScreen(isModallyPresented: true)
             }
         }
         .store(in: &cancellables)
@@ -126,6 +133,28 @@ class AuthenticationCoordinator: CoordinatorProtocol {
         navigationStackCoordinator.push(coordinator)
     }
     
+    private func showOIDCAuthentication(presentationAnchor: UIWindow) {
+        startLoading()
+        
+        Task {
+            switch await authenticationService.urlForOIDCLogin() {
+            case .failure(let error):
+                stopLoading()
+                handleError(error)
+            case .success(let oidcData):
+                stopLoading()
+                
+                let presenter = OIDCAuthenticationPresenter(authenticationService: authenticationService, presentationAnchor: presentationAnchor)
+                switch await presenter.authenticate(using: oidcData) {
+                case .success(let userSession):
+                    userHasSignedIn(userSession: userSession)
+                case .failure(let error):
+                    handleError(error)
+                }
+            }
+        }
+    }
+    
     private func showLoginScreen() {
         let parameters = LoginScreenCoordinatorParameters(authenticationService: authenticationService)
         let coordinator = LoginScreenCoordinator(parameters: parameters)
@@ -135,7 +164,10 @@ class AuthenticationCoordinator: CoordinatorProtocol {
 
             switch action {
             case .signedIn(let userSession):
-                self.userHasSignedIn(userSession: userSession)
+                userHasSignedIn(userSession: userSession)
+            case .configuredForOIDC:
+                // Pop back to the confirmation screen for OIDC login to continue.
+                navigationStackCoordinator.pop(animated: false)
             }
         }
 
@@ -150,7 +182,7 @@ class AuthenticationCoordinator: CoordinatorProtocol {
     }
 
     private func showAnalyticsPromptIfNeeded(completion: @escaping () -> Void) {
-        guard ServiceLocator.shared.analytics.shouldShowAnalyticsPrompt else {
+        guard serviceLocator.analytics.shouldShowAnalyticsPrompt else {
             completion()
             return
         }
@@ -164,13 +196,31 @@ class AuthenticationCoordinator: CoordinatorProtocol {
     private static let loadingIndicatorIdentifier = "AuthenticationCoordinatorLoading"
     
     private func startLoading() {
-        ServiceLocator.shared.userIndicatorController.submitIndicator(UserIndicator(id: Self.loadingIndicatorIdentifier,
-                                                                                    type: .modal,
-                                                                                    title: L10n.commonLoading,
-                                                                                    persistent: true))
+        serviceLocator.userIndicatorController.submitIndicator(UserIndicator(id: Self.loadingIndicatorIdentifier,
+                                                                             type: .modal,
+                                                                             title: L10n.commonLoading,
+                                                                             persistent: true))
     }
     
     private func stopLoading() {
-        ServiceLocator.shared.userIndicatorController.retractIndicatorWithId(Self.loadingIndicatorIdentifier)
+        serviceLocator.userIndicatorController.retractIndicatorWithId(Self.loadingIndicatorIdentifier)
+    }
+    
+    /// Processes an error to either update the flow or display it to the user.
+    private func handleError(_ error: AuthenticationServiceError) {
+        MXLog.warning("Error occurred: \(error)")
+        
+        switch error {
+        case .oidcError(.notSupported):
+            // Temporary alert hijacking the use of .notSupported, can be removed when OIDC support is in the SDK.
+            serviceLocator.userIndicatorController.alertInfo = AlertInfo(id: UUID(),
+                                                                         title: L10n.commonError,
+                                                                         message: L10n.commonServerNotSupported)
+        case .oidcError(.userCancellation):
+            // No need to show an error, the user cancelled authentication.
+            break
+        default:
+            serviceLocator.userIndicatorController.alertInfo = AlertInfo(id: UUID())
+        }
     }
 }

ElementX/Sources/Screens/Authentication/LoginScreen/LoginScreenCoordinator.swift

@@ -23,6 +23,8 @@ struct LoginScreenCoordinatorParameters {
 }
 
 enum LoginScreenCoordinatorAction {
+    /// The homeserver was updated to one that supports OIDC.
+    case configuredForOIDC
     /// Login was successful.
     case signedIn(UserSessionProtocol)
 }
@@ -33,7 +35,6 @@ final class LoginScreenCoordinator: CoordinatorProtocol {
     
     @CancellableTask private var currentTask: Task<Void, Error>?
     
-    private let oidcAuthenticationPresenter: OIDCAuthenticationPresenter
     private var authenticationService: AuthenticationServiceProxyProtocol { parameters.authenticationService }
 
     var callback: (@MainActor (LoginScreenCoordinatorAction) -> Void)?
@@ -44,8 +45,6 @@ final class LoginScreenCoordinator: CoordinatorProtocol {
         self.parameters = parameters
         
         viewModel = LoginScreenViewModel(homeserver: parameters.authenticationService.homeserver.value)
-        
-        oidcAuthenticationPresenter = OIDCAuthenticationPresenter(authenticationService: parameters.authenticationService)
     }
     
     // MARK: - Public
@@ -56,13 +55,11 @@ final class LoginScreenCoordinator: CoordinatorProtocol {
             
             switch action {
             case .parseUsername(let username):
-                self.parseUsername(username)
+                parseUsername(username)
             case .forgotPassword:
-                self.showForgotPasswordScreen()
+                showForgotPasswordScreen()
             case .login(let username, let password):
-                self.login(username: username, password: password)
-            case .continueWithOIDC:
-                self.continueWithOIDC()
+                login(username: username, password: password)
             }
         }
     }
@@ -114,38 +111,11 @@ final class LoginScreenCoordinator: CoordinatorProtocol {
             viewModel.displayError(.alert(L10n.screenLoginErrorDeactivatedAccount))
         case .slidingSyncNotAvailable:
             viewModel.displayError(.slidingSyncAlert)
-        case .oidcError(.notSupported):
-            // Temporary alert hijacking the use of .notSupported, can be removed when OIDC support is in the SDK.
-            viewModel.displayError(.alert(L10n.commonServerNotSupported))
-        case .oidcError(.userCancellation):
-            // No need to show an error, the user cancelled authentication.
-            break
         default:
             viewModel.displayError(.alert(L10n.errorUnknown))
         }
     }
     
-    private func continueWithOIDC() {
-        startLoading(isInteractionBlocking: true)
-        
-        Task {
-            switch await authenticationService.urlForOIDCLogin() {
-            case .failure(let error):
-                stopLoading()
-                handleError(error)
-            case .success(let oidcData):
-                stopLoading()
-                
-                switch await oidcAuthenticationPresenter.authenticate(using: oidcData) {
-                case .success(let userSession):
-                    callback?(.signedIn(userSession))
-                case .failure(let error):
-                    handleError(error)
-                }
-            }
-        }
-    }
-    
     /// Requests the authentication coordinator to log in using the specified credentials.
     private func login(username: String, password: String) {
         MXLog.info("Starting login with password.")
@@ -177,8 +147,12 @@ final class LoginScreenCoordinator: CoordinatorProtocol {
         Task {
             switch await authenticationService.configure(for: homeserverDomain) {
             case .success:
-                updateViewModel()
                 stopLoading()
+                if authenticationService.homeserver.value.loginMode == .oidc {
+                    callback?(.configuredForOIDC)
+                } else {
+                    updateViewModel()
+                }
             case .failure(let error):
                 stopLoading()
                 handleError(error)

ElementX/Sources/Screens/Authentication/LoginScreen/LoginScreenModels.swift

@@ -23,8 +23,6 @@ enum LoginScreenViewModelAction: CustomStringConvertible {
     case forgotPassword
     /// Login using the supplied credentials.
     case login(username: String, password: String)
-    /// Continue using OIDC.
-    case continueWithOIDC
     
     /// A string representation of the action, ignoring any associated values that could leak PII.
     var description: String {
@@ -35,8 +33,6 @@ enum LoginScreenViewModelAction: CustomStringConvertible {
             return "forgotPassword"
         case .login:
             return "login"
-        case .continueWithOIDC:
-            return "continueWithOIDC"
         }
     }
 }
@@ -79,8 +75,6 @@ enum LoginScreenViewAction {
     case forgotPassword
     /// Continue using the input username and password.
     case next
-    /// Continue using OIDC.
-    case continueWithOIDC
 }
 
 enum LoginScreenErrorType: Hashable {

ElementX/Sources/Screens/Authentication/LoginScreen/LoginScreenViewModel.swift

@@ -36,8 +36,6 @@ class LoginScreenViewModel: LoginScreenViewModelType, LoginScreenViewModelProtoc
             callback?(.forgotPassword)
         case .next:
             callback?(.login(username: state.bindings.username, password: state.bindings.password))
-        case .continueWithOIDC:
-            callback?(.continueWithOIDC)
         }
     }
     

ElementX/Sources/Screens/Authentication/LoginScreen/View/LoginScreen.swift

@@ -35,7 +35,8 @@ struct LoginScreen: View {
                 case .password:
                     loginForm
                 case .oidc:
-                    oidcButton
+                    // This should never be shown.
+                    ProgressView()
                 default:
                     loginUnavailableText
                 }
@@ -105,15 +106,6 @@ struct LoginScreen: View {
             .accessibilityIdentifier(A11yIdentifiers.loginScreen.continue)
         }
     }
-
-    /// The OIDC button that can be used for login.
-    var oidcButton: some View {
-        Button { context.send(viewAction: .continueWithOIDC) } label: {
-            Text(L10n.actionContinue)
-        }
-        .buttonStyle(.elementAction(.xLarge))
-        .accessibilityIdentifier(A11yIdentifiers.loginScreen.oidc)
-    }
     
     /// Text shown if neither password or OIDC login is supported.
     var loginUnavailableText: some View {
@@ -155,10 +147,10 @@ struct LoginScreen_Previews: PreviewProvider {
             .previewDisplayName("matrix.org")
         screen(for: credentialsViewModel)
             .previewDisplayName("Credentials Entered")
-        screen(for: LoginScreenViewModel(homeserver: .mockOIDC))
-            .previewDisplayName("OIDC")
         screen(for: LoginScreenViewModel(homeserver: .mockUnsupported))
             .previewDisplayName("Unsupported")
+        screen(for: LoginScreenViewModel(homeserver: .mockOIDC))
+            .previewDisplayName("OIDC Fallback")
     }
     
     static func screen(for viewModel: LoginScreenViewModel) -> some View {

ElementX/Sources/Screens/Authentication/OIDCAuthenticationPresenter.swift

@@ -17,12 +17,15 @@
 import AuthenticationServices
 
 /// Presents a web authentication session for an OIDC request.
-class OIDCAuthenticationPresenter {
+@MainActor
+class OIDCAuthenticationPresenter: NSObject {
     private let authenticationService: AuthenticationServiceProxyProtocol
-    private let oidcPresentationContent = OIDCPresentationContext()
+    private let presentationAnchor: UIWindow
     
-    init(authenticationService: AuthenticationServiceProxyProtocol) {
+    init(authenticationService: AuthenticationServiceProxyProtocol, presentationAnchor: UIWindow) {
         self.authenticationService = authenticationService
+        self.presentationAnchor = presentationAnchor
+        super.init()
     }
     
     /// Presents a web authentication session for the supplied data.
@@ -48,7 +51,7 @@ class OIDCAuthenticationPresenter {
             }
             
             session.prefersEphemeralWebBrowserSession = false
-            session.presentationContextProvider = oidcPresentationContent
+            session.presentationContextProvider = self
             session.start()
         }
     }
@@ -67,11 +70,8 @@ class OIDCAuthenticationPresenter {
     }
 }
 
-class OIDCPresentationContext: NSObject, ASWebAuthenticationPresentationContextProviding {
-    func presentationAnchor(for session: ASWebAuthenticationSession) -> ASPresentationAnchor {
-        guard let window = UIApplication.shared.connectedScenes.compactMap({ $0 as? UIWindowScene }).first?.keyWindow else {
-            fatalError("Failed to find the main window.")
-        }
-        return window
-    }
+// MARK: ASWebAuthenticationPresentationContextProviding
+
+extension OIDCAuthenticationPresenter: ASWebAuthenticationPresentationContextProviding {
+    func presentationAnchor(for session: ASWebAuthenticationSession) -> ASPresentationAnchor { presentationAnchor }
 }

ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenCoordinator.swift

@@ -23,7 +23,7 @@ struct ServerConfirmationScreenCoordinatorParameters {
 }
 
 enum ServerConfirmationScreenCoordinatorAction {
-    case confirm
+    case `continue`(UIWindow?)
     case changeServer
 }
 
@@ -50,7 +50,7 @@ final class ServerConfirmationScreenCoordinator: CoordinatorProtocol {
             
             switch action {
             case .confirm:
-                self.actionsSubject.send(.confirm)
+                self.actionsSubject.send(.continue(viewModel.context.viewState.window))
             case .changeServer:
                 self.actionsSubject.send(.changeServer)
             }

ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenModels.swift

@@ -14,7 +14,7 @@
 // limitations under the License.
 //
 
-import Foundation
+import SwiftUI
 
 enum ServerConfirmationScreenViewModelAction {
     /// The user would like to continue with the current homeserver.
@@ -28,6 +28,8 @@ struct ServerConfirmationScreenViewState: BindableState {
     var homeserverAddress: String
     /// The flow being attempted on the selected homeserver.
     let authenticationFlow: AuthenticationFlow
+    /// The presentation anchor used for OIDC authentication.
+    var window: UIWindow?
     
     /// The screen's title.
     var title: String {
@@ -57,6 +59,8 @@ struct ServerConfirmationScreenViewState: BindableState {
 }
 
 enum ServerConfirmationScreenViewAction {
+    /// Updates the window used as the OIDC presentation anchor.
+    case updateWindow(UIWindow)
     /// The user would like to continue with the current homeserver.
     case confirm
     /// The user would like to change to a different homeserver.

ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenViewModel.swift

@@ -43,6 +43,9 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
     
     override func process(viewAction: ServerConfirmationScreenViewAction) {
         switch viewAction {
+        case .updateWindow(let window):
+            guard state.window != window else { return }
+            Task { state.window = window }
         case .confirm:
             actionsSubject.send(.confirm)
         case .changeServer:

ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/View/ServerConfirmationScreen.swift

@@ -34,6 +34,10 @@ struct ServerConfirmationScreen: View {
                 .readableFrame()
                 .background(Color.element.background.ignoresSafeArea())
         }
+        .introspectViewController { viewController in
+            guard let window = viewController.view.window else { return }
+            context.send(viewAction: .updateWindow(window))
+        }
     }
     
     /// The main content of the view to be shown in a scroll view.

ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenCoordinator.swift

@@ -43,7 +43,6 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
     private let parameters: SoftLogoutScreenCoordinatorParameters
     private var viewModel: SoftLogoutScreenViewModelProtocol
     
-    private let oidcAuthenticationPresenter: OIDCAuthenticationPresenter
     private var authenticationService: AuthenticationServiceProxyProtocol { parameters.authenticationService }
     
     var callback: (@MainActor (SoftLogoutScreenCoordinatorResult) -> Void)?
@@ -55,8 +54,6 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
         viewModel = SoftLogoutScreenViewModel(credentials: parameters.credentials,
                                               homeserver: homeserver.value,
                                               keyBackupNeeded: parameters.keyBackupNeeded)
-        
-        oidcAuthenticationPresenter = OIDCAuthenticationPresenter(authenticationService: parameters.authenticationService)
     }
     
     // MARK: - Public
@@ -74,7 +71,7 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
             case .clearAllData:
                 self.callback?(.clearAllData)
             case .continueWithOIDC:
-                self.continueWithOIDC()
+                self.continueWithOIDC(presentationAnchor: viewModel.context.viewState.window)
             }
         }
     }
@@ -130,7 +127,9 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
         }
     }
 
-    private func continueWithOIDC() {
+    private func continueWithOIDC(presentationAnchor: UIWindow?) {
+        guard let presentationAnchor else { return }
+        
         startLoading()
         
         Task {
@@ -141,7 +140,8 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
             case .success(let oidcData):
                 stopLoading()
                 
-                switch await oidcAuthenticationPresenter.authenticate(using: oidcData) {
+                let presenter = OIDCAuthenticationPresenter(authenticationService: parameters.authenticationService, presentationAnchor: presentationAnchor)
+                switch await presenter.authenticate(using: oidcData) {
                 case .success(let userSession):
                     callback?(.signedIn(userSession))
                 case .failure(let error):

ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenModels.swift

@@ -63,6 +63,9 @@ struct SoftLogoutScreenViewState: BindableState {
 
     /// The types of login supported by the homeserver.
     var loginMode: LoginMode { homeserver.loginMode }
+    
+    /// The presentation anchor used for OIDC authentication.
+    var window: UIWindow?
 
     /// Whether to show recover encryption keys message
     var showRecoverEncryptionKeysMessage: Bool {
@@ -83,6 +86,8 @@ struct SoftLogoutScreenBindings {
 }
 
 enum SoftLogoutScreenViewAction {
+    /// Updates the window used as the OIDC presentation anchor.
+    case updateWindow(UIWindow?)
     /// Login.
     case login
     /// Forgot password

ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenViewModel.swift

@@ -43,6 +43,9 @@ class SoftLogoutScreenViewModel: SoftLogoutScreenViewModelType, SoftLogoutScreen
             callback?(.clearAllData)
         case .continueWithOIDC:
             callback?(.continueWithOIDC)
+        case .updateWindow(let window):
+            guard state.window != window else { return }
+            Task { state.window = window }
         }
     }
 

ElementX/Sources/Screens/Authentication/SoftLogoutScreen/View/SoftLogoutScreen.swift

@@ -49,6 +49,10 @@ struct SoftLogoutScreen: View {
         }
         .background(Color.element.background.ignoresSafeArea())
         .alert(item: $context.alertInfo) { $0.alert }
+        .introspectViewController { viewController in
+            guard let window = viewController.view.window else { return }
+            context.send(viewAction: .updateWindow(window))
+        }
     }
 
     /// The title, message and icon at the top of the screen.

UITests/Sources/LoginScreenUITests.swift

@@ -32,17 +32,6 @@ class LoginScreenUITests: XCTestCase {
         try await app.assertScreenshot(.login, step: 0)
     }
     
-    func testOIDC() async throws {
-        // Given the initial login screen.
-        let app = Application.launch(.login)
-        
-        // When entering a username on a homeserver that only supports OIDC.
-        app.textFields[A11yIdentifiers.loginScreen.emailUsername].clearAndTypeText("@test:company.com\n")
-        
-        // Then the screen should be configured for OIDC.
-        try await app.assertScreenshot(.login, step: 1)
-    }
-    
     func testUnsupported() async throws {
         // Given the initial login screen.
         let app = Application.launch(.login)
@@ -51,6 +40,6 @@ class LoginScreenUITests: XCTestCase {
         app.textFields[A11yIdentifiers.loginScreen.emailUsername].clearAndTypeText("@test:server.net\n")
         
         // Then the screen should not allow login to continue.
-        try await app.assertScreenshot(.login, step: 2)
+        try await app.assertScreenshot(.login, step: 1)
     }
 }

UITests/Sources/ServerSelectionUITests.swift

@@ -48,7 +48,7 @@ class ServerSelectionUITests: XCTestCase {
         
         // Then an error should be shown and the confirmation button disabled.
         try await app.assertScreenshot(.serverSelection, step: 2)
-        XCTAssertFalse(app.buttons[A11yIdentifiers.changeServerScreen.continue].isEnabled, "The confirm button should be disabled when there is an error.")
+        XCTAssertFalse(app.buttons[A11yIdentifiers.changeServerScreen.continue].isEnabled, "The continue button should be disabled when there is an error.")
     }
 
     func testNonModalPresentation() async throws {