mirror of
https://github.com/Novaloop-AG/element-x-ios.git
synced 2026-11-02 21:33:26 +00:00
Fix authentication activity indicators when the authentication server is being slow
* Fetch the OIDC login URL as part of the Server Confirmation screen. The loading indicator would show and hide while configuring and then immediately show again while fetching the URL. * Allow the OIDCAuthenticationPresenter to show its own errors. We no longer need to present errors in the authentication flow coordinator.
This commit is contained in:
@@ -144,12 +144,10 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
|
||||
guard let self else { return }
|
||||
|
||||
switch action {
|
||||
case .continue(let window):
|
||||
if authenticationService.homeserver.value.loginMode.supportsOIDCFlow, let window {
|
||||
showOIDCAuthentication(presentationAnchor: window)
|
||||
} else {
|
||||
showLoginScreen()
|
||||
}
|
||||
case .continueWithOIDC(let oidcData, let window):
|
||||
showOIDCAuthentication(oidcData: oidcData, presentationAnchor: window)
|
||||
case .continueWithPassword:
|
||||
showLoginScreen()
|
||||
case .changeServer:
|
||||
showServerSelectionScreen(authenticationFlow: authenticationFlow)
|
||||
}
|
||||
@@ -185,29 +183,21 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
|
||||
navigationStackCoordinator.setSheetCoordinator(navigationCoordinator)
|
||||
}
|
||||
|
||||
private func showOIDCAuthentication(presentationAnchor: UIWindow) {
|
||||
startLoading()
|
||||
private func showOIDCAuthentication(oidcData: OIDCAuthorizationDataProxy, presentationAnchor: UIWindow) {
|
||||
let presenter = OIDCAuthenticationPresenter(authenticationService: authenticationService,
|
||||
oidcRedirectURL: appSettings.oidcRedirectURL,
|
||||
presentationAnchor: presentationAnchor,
|
||||
userIndicatorController: userIndicatorController)
|
||||
oidcPresenter = presenter
|
||||
|
||||
Task {
|
||||
switch await authenticationService.urlForOIDCLogin() {
|
||||
case .failure(let error):
|
||||
stopLoading()
|
||||
handleError(error)
|
||||
case .success(let oidcData):
|
||||
stopLoading()
|
||||
|
||||
let presenter = OIDCAuthenticationPresenter(authenticationService: authenticationService,
|
||||
oidcRedirectURL: appSettings.oidcRedirectURL,
|
||||
presentationAnchor: presentationAnchor)
|
||||
self.oidcPresenter = presenter
|
||||
switch await presenter.authenticate(using: oidcData) {
|
||||
case .success(let userSession):
|
||||
userHasSignedIn(userSession: userSession)
|
||||
case .failure(let error):
|
||||
handleError(error)
|
||||
}
|
||||
oidcPresenter = nil
|
||||
switch await presenter.authenticate(using: oidcData) {
|
||||
case .success(let userSession):
|
||||
userHasSignedIn(userSession: userSession)
|
||||
case .failure:
|
||||
break // Nothing to do, the alerts are handled by the presenter.
|
||||
}
|
||||
oidcPresenter = nil
|
||||
}
|
||||
}
|
||||
|
||||
@@ -238,35 +228,4 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol {
|
||||
private func userHasSignedIn(userSession: UserSessionProtocol) {
|
||||
delegate?.authenticationFlowCoordinator(didLoginWithSession: userSession)
|
||||
}
|
||||
|
||||
private static let loadingIndicatorIdentifier = "\(AuthenticationFlowCoordinator.self)-Loading"
|
||||
|
||||
private func startLoading() {
|
||||
userIndicatorController.submitIndicator(UserIndicator(id: Self.loadingIndicatorIdentifier,
|
||||
type: .modal,
|
||||
title: L10n.commonLoading,
|
||||
persistent: true))
|
||||
}
|
||||
|
||||
private func stopLoading() {
|
||||
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.
|
||||
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:
|
||||
userIndicatorController.alertInfo = AlertInfo(id: UUID())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -139,7 +139,7 @@ struct LoginScreen_Previews: PreviewProvider, TestablePreview {
|
||||
.snapshotPreferences(expect: viewModel.context.$viewState.map { state in
|
||||
state.homeserver.loginMode == .password
|
||||
})
|
||||
.previewDisplayName("matrix.org")
|
||||
.previewDisplayName("Initial State")
|
||||
|
||||
NavigationStack {
|
||||
LoginScreen(context: credentialsViewModel.context)
|
||||
@@ -155,7 +155,7 @@ struct LoginScreen_Previews: PreviewProvider, TestablePreview {
|
||||
.previewDisplayName("Unsupported")
|
||||
}
|
||||
|
||||
static func makeViewModel(homeserverAddress: String = "matrix.org", withCredentials: Bool = false) -> LoginScreenViewModel {
|
||||
static func makeViewModel(homeserverAddress: String = "example.com", withCredentials: Bool = false) -> LoginScreenViewModel {
|
||||
let authenticationService = AuthenticationService.mock
|
||||
|
||||
Task { await authenticationService.configure(for: homeserverAddress, flow: .login) }
|
||||
|
||||
@@ -13,11 +13,16 @@ class OIDCAuthenticationPresenter: NSObject {
|
||||
private let authenticationService: AuthenticationServiceProtocol
|
||||
private let oidcRedirectURL: URL
|
||||
private let presentationAnchor: UIWindow
|
||||
private let userIndicatorController: UserIndicatorControllerProtocol
|
||||
|
||||
init(authenticationService: AuthenticationServiceProtocol, oidcRedirectURL: URL, presentationAnchor: UIWindow) {
|
||||
init(authenticationService: AuthenticationServiceProtocol,
|
||||
oidcRedirectURL: URL,
|
||||
presentationAnchor: UIWindow,
|
||||
userIndicatorController: UserIndicatorControllerProtocol) {
|
||||
self.authenticationService = authenticationService
|
||||
self.oidcRedirectURL = oidcRedirectURL
|
||||
self.presentationAnchor = presentationAnchor
|
||||
self.userIndicatorController = userIndicatorController
|
||||
super.init()
|
||||
}
|
||||
|
||||
@@ -39,21 +44,44 @@ class OIDCAuthenticationPresenter: NSObject {
|
||||
if let nsError = error as? NSError,
|
||||
nsError.domain == ASWebAuthenticationSessionErrorDomain,
|
||||
nsError.code == ASWebAuthenticationSessionError.canceledLogin.rawValue {
|
||||
// No need to show an error here, just abort and return a failure.
|
||||
await authenticationService.abortOIDCLogin(data: oidcData)
|
||||
return .failure(.oidcError(.userCancellation))
|
||||
}
|
||||
|
||||
MXLog.error("Missing callback URL from the web authentication session.")
|
||||
userIndicatorController.alertInfo = AlertInfo(id: UUID())
|
||||
await authenticationService.abortOIDCLogin(data: oidcData)
|
||||
return .failure(.oidcError(.unknown))
|
||||
}
|
||||
|
||||
// Exchanging the callback with the homeserver can be slow, so show the loading indicator while we wait (the modal has already been dismissed).
|
||||
startLoading(delay: .milliseconds(50)) // Small delay to handle a cancellation callback without the indicator showing.
|
||||
defer { stopLoading() }
|
||||
|
||||
switch await authenticationService.loginWithOIDCCallback(url) {
|
||||
case .success(let userSession):
|
||||
return .success(userSession)
|
||||
case .failure(let error):
|
||||
MXLog.error("Error occurred: \(error)")
|
||||
userIndicatorController.alertInfo = AlertInfo(id: UUID())
|
||||
return .failure(error)
|
||||
}
|
||||
}
|
||||
|
||||
private static let loadingIndicatorID = "\(OIDCAuthenticationPresenter.self)-Loading"
|
||||
|
||||
private func startLoading(delay: Duration? = nil) {
|
||||
userIndicatorController.submitIndicator(UserIndicator(id: Self.loadingIndicatorID,
|
||||
type: .modal,
|
||||
title: L10n.commonLoading,
|
||||
persistent: true),
|
||||
delay: delay)
|
||||
}
|
||||
|
||||
private func stopLoading() {
|
||||
userIndicatorController.retractIndicatorWithId(Self.loadingIndicatorID)
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: ASWebAuthenticationPresentationContextProviding
|
||||
|
||||
+7
-4
@@ -16,7 +16,8 @@ struct ServerConfirmationScreenCoordinatorParameters {
|
||||
}
|
||||
|
||||
enum ServerConfirmationScreenCoordinatorAction {
|
||||
case `continue`(UIWindow?)
|
||||
case continueWithOIDC(data: OIDCAuthorizationDataProxy, window: UIWindow)
|
||||
case continueWithPassword
|
||||
case changeServer
|
||||
}
|
||||
|
||||
@@ -41,10 +42,12 @@ final class ServerConfirmationScreenCoordinator: CoordinatorProtocol {
|
||||
guard let self else { return }
|
||||
|
||||
switch action {
|
||||
case .confirm:
|
||||
self.actionsSubject.send(.continue(viewModel.context.viewState.window))
|
||||
case .continueWithOIDC(let oidcData, let window):
|
||||
actionsSubject.send(.continueWithOIDC(data: oidcData, window: window))
|
||||
case .continueWithPassword:
|
||||
actionsSubject.send(.continueWithPassword)
|
||||
case .changeServer:
|
||||
self.actionsSubject.send(.changeServer)
|
||||
actionsSubject.send(.changeServer)
|
||||
}
|
||||
}
|
||||
.store(in: &cancellables)
|
||||
|
||||
+4
-2
@@ -8,8 +8,10 @@
|
||||
import SwiftUI
|
||||
|
||||
enum ServerConfirmationScreenViewModelAction {
|
||||
/// The user would like to continue with the current homeserver.
|
||||
case confirm
|
||||
/// Continue the flow using the provided OIDC parameters.
|
||||
case continueWithOIDC(data: OIDCAuthorizationDataProxy, window: UIWindow)
|
||||
/// Continue the flow using password authentication.
|
||||
case continueWithPassword
|
||||
/// The user would like to change to a different homeserver.
|
||||
case changeServer
|
||||
}
|
||||
|
||||
+33
-8
@@ -48,7 +48,7 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
|
||||
guard state.window != window else { return }
|
||||
Task { state.window = window }
|
||||
case .confirm:
|
||||
Task { await configureAndContinue() }
|
||||
Task { await confirmHomeserver() }
|
||||
case .changeServer:
|
||||
actionsSubject.send(.changeServer)
|
||||
}
|
||||
@@ -56,13 +56,13 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
|
||||
|
||||
// MARK: - Private
|
||||
|
||||
private func configureAndContinue() async {
|
||||
private func confirmHomeserver() async {
|
||||
let homeserver = authenticationService.homeserver.value
|
||||
|
||||
// If the login mode is unknown, the service hasn't be configured and we need to do it now.
|
||||
// Otherwise we can continue the flow as server selection has been performed and succeeded.
|
||||
guard homeserver.loginMode == .unknown || authenticationService.flow != authenticationFlow else {
|
||||
actionsSubject.send(.confirm)
|
||||
await fetchLoginURLIfNeededAndContinue()
|
||||
return
|
||||
}
|
||||
|
||||
@@ -71,7 +71,7 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
|
||||
|
||||
switch await authenticationService.configure(for: homeserver.address, flow: authenticationFlow) {
|
||||
case .success:
|
||||
actionsSubject.send(.confirm)
|
||||
await fetchLoginURLIfNeededAndContinue()
|
||||
case .failure(let error):
|
||||
switch error {
|
||||
case .invalidServer, .invalidHomeserverAddress:
|
||||
@@ -90,14 +90,39 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
|
||||
}
|
||||
}
|
||||
|
||||
private func startLoading(label: String = L10n.commonLoading) {
|
||||
userIndicatorController.submitIndicator(UserIndicator(type: .modal,
|
||||
title: label,
|
||||
private func fetchLoginURLIfNeededAndContinue() async {
|
||||
guard authenticationService.homeserver.value.loginMode.supportsOIDCFlow else {
|
||||
actionsSubject.send(.continueWithPassword)
|
||||
return
|
||||
}
|
||||
|
||||
guard let window = state.window else {
|
||||
displayError(.unknownError)
|
||||
return
|
||||
}
|
||||
|
||||
startLoading() // Uses the same ID, so no need to worry if the indicator already exists
|
||||
defer { stopLoading() }
|
||||
|
||||
switch await authenticationService.urlForOIDCLogin() {
|
||||
case .success(let oidcData):
|
||||
actionsSubject.send(.continueWithOIDC(data: oidcData, window: window))
|
||||
case .failure:
|
||||
displayError(.unknownError)
|
||||
}
|
||||
}
|
||||
|
||||
private let loadingIndicatorID = "\(ServerConfirmationScreenViewModel.self)-Loading"
|
||||
|
||||
private func startLoading() {
|
||||
userIndicatorController.submitIndicator(UserIndicator(id: loadingIndicatorID,
|
||||
type: .modal,
|
||||
title: L10n.commonLoading,
|
||||
persistent: true))
|
||||
}
|
||||
|
||||
private func stopLoading() {
|
||||
userIndicatorController.retractAllIndicators()
|
||||
userIndicatorController.retractIndicatorWithId(loadingIndicatorID)
|
||||
}
|
||||
|
||||
private func displayError(_ type: ServerConfirmationScreenAlert) {
|
||||
|
||||
+3
-3
@@ -29,11 +29,11 @@ enum MockSoftLogoutScreenState: String, CaseIterable {
|
||||
switch self {
|
||||
case .emptyPassword:
|
||||
return SoftLogoutScreenViewModel(credentials: credentials,
|
||||
homeserver: .mockMatrixDotOrg,
|
||||
homeserver: .mockBasicServer,
|
||||
keyBackupNeeded: false)
|
||||
case .enteredPassword:
|
||||
return SoftLogoutScreenViewModel(credentials: credentials,
|
||||
homeserver: .mockMatrixDotOrg,
|
||||
homeserver: .mockBasicServer,
|
||||
keyBackupNeeded: false,
|
||||
password: "12345678")
|
||||
case .oidc:
|
||||
@@ -46,7 +46,7 @@ enum MockSoftLogoutScreenState: String, CaseIterable {
|
||||
keyBackupNeeded: false)
|
||||
case .keyBackupNeeded:
|
||||
return SoftLogoutScreenViewModel(credentials: credentials,
|
||||
homeserver: .mockMatrixDotOrg,
|
||||
homeserver: .mockBasicServer,
|
||||
keyBackupNeeded: true)
|
||||
}
|
||||
}
|
||||
|
||||
+2
-1
@@ -143,7 +143,8 @@ final class SoftLogoutScreenCoordinator: CoordinatorProtocol {
|
||||
|
||||
let presenter = OIDCAuthenticationPresenter(authenticationService: parameters.authenticationService,
|
||||
oidcRedirectURL: ServiceLocator.shared.settings.oidcRedirectURL,
|
||||
presentationAnchor: presentationAnchor)
|
||||
presentationAnchor: presentationAnchor,
|
||||
userIndicatorController: parameters.userIndicatorController)
|
||||
self.oidcPresenter = presenter
|
||||
switch await presenter.authenticate(using: oidcData) {
|
||||
case .success(let userSession):
|
||||
|
||||
Reference in New Issue
Block a user