diff --git a/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift index 93b6a1bbd..55896596a 100644 --- a/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift @@ -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()) - } - } } diff --git a/ElementX/Sources/Screens/Authentication/LoginScreen/View/LoginScreen.swift b/ElementX/Sources/Screens/Authentication/LoginScreen/View/LoginScreen.swift index d43b3d999..857ff8059 100644 --- a/ElementX/Sources/Screens/Authentication/LoginScreen/View/LoginScreen.swift +++ b/ElementX/Sources/Screens/Authentication/LoginScreen/View/LoginScreen.swift @@ -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) } diff --git a/ElementX/Sources/Screens/Authentication/OIDCAuthenticationPresenter.swift b/ElementX/Sources/Screens/Authentication/OIDCAuthenticationPresenter.swift index 9d0d97d77..9c352ebc8 100644 --- a/ElementX/Sources/Screens/Authentication/OIDCAuthenticationPresenter.swift +++ b/ElementX/Sources/Screens/Authentication/OIDCAuthenticationPresenter.swift @@ -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 diff --git a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenCoordinator.swift b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenCoordinator.swift index 240b08862..bea05cb8e 100644 --- a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenCoordinator.swift +++ b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenCoordinator.swift @@ -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) diff --git a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenModels.swift b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenModels.swift index 228982e98..5ce1c80bf 100644 --- a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenModels.swift +++ b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenModels.swift @@ -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 } diff --git a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenViewModel.swift b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenViewModel.swift index f238c7696..451fbffc3 100644 --- a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenViewModel.swift +++ b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenViewModel.swift @@ -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) { diff --git a/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/MockSoftLogoutScreenState.swift b/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/MockSoftLogoutScreenState.swift index 78d4b53ed..738fd909f 100644 --- a/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/MockSoftLogoutScreenState.swift +++ b/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/MockSoftLogoutScreenState.swift @@ -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) } } diff --git a/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenCoordinator.swift b/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenCoordinator.swift index 9fb2cd357..9c171f2a3 100644 --- a/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenCoordinator.swift +++ b/ElementX/Sources/Screens/Authentication/SoftLogoutScreen/SoftLogoutScreenCoordinator.swift @@ -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): diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPad-en-GB.png index cd9ba73da..14bf9deee 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPad-en-GB.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPad-en-GB.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:5eb36898636e84943c7b1b2f3ef0289bf133b4da57f4ce3a1ba9c387ce5ea91c -size 91146 +oid sha256:2be2a30d2c8204d2ef860e79877bd277b899c0725560f9abf47eb7f5a929f187 +size 92468 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPad-pseudo.png index 3d7a43b40..628ac1a5e 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPad-pseudo.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPad-pseudo.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:eb17164648a312eb4a32d91c8908b99d69ec7dc3265b1af7eb91514bbaf74ac6 -size 95333 +oid sha256:662b49aa4f0466a52723ad538e507604b19b70c551e1b912296cc7d4f82cfeb4 +size 96651 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPhone-16-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPhone-16-en-GB.png index 0090f403f..8a4d7cb96 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPhone-16-en-GB.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPhone-16-en-GB.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:324939f6d163efe36c492f67b74b09e56f71948b518e3e0910b11124671b82d7 -size 47333 +oid sha256:845f7d62abf84d40c8913befa23053423b1fa1c9b2d4a3f173e57e399181c6c3 +size 48439 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPhone-16-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPhone-16-pseudo.png index c59284ad5..6229df2a7 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPhone-16-pseudo.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Credentials-Entered-iPhone-16-pseudo.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:0fb309bbd4129b1ad027c8e8ee302a2815adf7c58f9e5de8398b9ce0420851f4 -size 54972 +oid sha256:b8a14e13a1687c5b1fdeb9a9149105cbf096ff17ff56e19f93bf77d3ba91202b +size 55382 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPad-en-GB.png new file mode 100644 index 000000000..8b46f9a1e --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPad-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:036ac6ac02cff4007cc762a2f0bb1c4f5fa34a15ede708147aecd249a2e69587 +size 95915 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPad-pseudo.png new file mode 100644 index 000000000..8be38341d --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPad-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:7fc634e0b161123055a8ddf81269882b37c511b6b8feee0d0b86309350dfae78 +size 100917 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPhone-16-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPhone-16-en-GB.png new file mode 100644 index 000000000..913b238c6 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPhone-16-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:47ab21f12dbcdd0ffeb7da8aa0aaf3ed409e7d9f2255979f96a621e41a2206ba +size 51503 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPhone-16-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPhone-16-pseudo.png new file mode 100644 index 000000000..1cdf5f4fd --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.Initial-State-iPhone-16-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:650ebf41da7dc3c084144e00aa4aaf4837ca2ede976cdc5d4f7c6887ea921ac8 +size 59766 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPad-en-GB.png deleted file mode 100644 index 9a13d1f33..000000000 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPad-en-GB.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:8c13f24fb140219627c51b1d680149065901b39d24b773525af663b2c42f536e -size 94615 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPad-pseudo.png deleted file mode 100644 index f028e20a5..000000000 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPad-pseudo.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:d277215e66e5b80c96386c819a9a82b5f360cb6639d17b16783cf27ea6390fdf -size 99601 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPhone-16-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPhone-16-en-GB.png deleted file mode 100644 index 71bf61c08..000000000 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPhone-16-en-GB.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:97ac8c4c12de6ccca253c59eaa9ac0baf6243094d2ed9691ff9ed1cb363fa42a -size 50411 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPhone-16-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPhone-16-pseudo.png deleted file mode 100644 index 20095a57d..000000000 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/loginScreen.matrix-org-iPhone-16-pseudo.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:c0216e37d95512a2f03bb4fdc2cf903ecc05d62400e642ed9e6cadb8590af24e -size 59387 diff --git a/UITests/Sources/AuthenticationFlowCoordinatorTests.swift b/UITests/Sources/AuthenticationFlowCoordinatorTests.swift index 94d136664..24b179ba1 100644 --- a/UITests/Sources/AuthenticationFlowCoordinatorTests.swift +++ b/UITests/Sources/AuthenticationFlowCoordinatorTests.swift @@ -16,6 +16,12 @@ class AuthenticationFlowCoordinatorUITests: XCTestCase { // Splash Screen: Tap get started button app.buttons[A11yIdentifiers.authenticationStartScreen.signIn].tap() + // Server Confirmation: Tap change server button + app.buttons[A11yIdentifiers.serverConfirmationScreen.changeServer].tap() + + // Server Selection: Clear the default, enter OIDC server and continue. + app.textFields[A11yIdentifiers.changeServerScreen.server].clearAndTypeText("example.com\n", app: app) + // Server Confirmation: Tap continue button app.buttons[A11yIdentifiers.serverConfirmationScreen.continue].tap() @@ -40,6 +46,12 @@ class AuthenticationFlowCoordinatorUITests: XCTestCase { // Splash Screen: Tap get started button app.buttons[A11yIdentifiers.authenticationStartScreen.signIn].tap() + // Server Confirmation: Tap change server button + app.buttons[A11yIdentifiers.serverConfirmationScreen.changeServer].tap() + + // Server Selection: Clear the default, enter OIDC server and continue. + app.textFields[A11yIdentifiers.changeServerScreen.server].clearAndTypeText("example.com\n", app: app) + // Server Confirmation: Tap continue button app.buttons[A11yIdentifiers.serverConfirmationScreen.continue].tap() @@ -66,6 +78,12 @@ class AuthenticationFlowCoordinatorUITests: XCTestCase { // Splash Screen: Tap get started button app.buttons[A11yIdentifiers.authenticationStartScreen.signIn].tap() + // Server Confirmation: Tap change server button + app.buttons[A11yIdentifiers.serverConfirmationScreen.changeServer].tap() + + // Server Selection: Clear the default, enter OIDC server and continue. + app.textFields[A11yIdentifiers.changeServerScreen.server].clearAndTypeText("example.com\n", app: app) + // Server Confirmation: Tap continue button app.buttons[A11yIdentifiers.serverConfirmationScreen.continue].tap() @@ -96,7 +114,8 @@ class AuthenticationFlowCoordinatorUITests: XCTestCase { // Server Confirmation: Tap continue button app.buttons[A11yIdentifiers.serverConfirmationScreen.continue].tap() - // Then the login form shouldn't be shown as OIDC will be used instead. - XCTAssertFalse(app.buttons[A11yIdentifiers.loginScreen.continue].waitForExistence(timeout: 1), "The login screen should not be shown after selecting a homeserver with OIDC.") + let springboard = XCUIApplication(bundleIdentifier: "com.apple.springboard") + XCTAssertTrue(springboard.staticTexts["“ElementX” Wants to Use “company.com” to Sign In"].waitForExistence(timeout: 2), + "The web authentication prompt should be shown after selecting a homeserver with OIDC.") } } diff --git a/UnitTests/Sources/ServerConfirmationScreenViewModelTests.swift b/UnitTests/Sources/ServerConfirmationScreenViewModelTests.swift index 75a1a3220..1de669823 100644 --- a/UnitTests/Sources/ServerConfirmationScreenViewModelTests.swift +++ b/UnitTests/Sources/ServerConfirmationScreenViewModelTests.swift @@ -12,6 +12,7 @@ import XCTest @MainActor class ServerConfirmationScreenViewModelTests: XCTestCase { var clientBuilderFactory: AuthenticationClientBuilderFactoryMock! + var client: ClientSDKMock! var service: AuthenticationServiceProtocol! var viewModel: ServerConfirmationScreenViewModel! @@ -22,15 +23,18 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { setupViewModel(authenticationFlow: .login) XCTAssertEqual(service.homeserver.value.loginMode, .unknown) XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 0) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 0) // When continuing from the confirmation screen. - let deferred = deferFulfillment(viewModel.actions) { $0 == .confirm } + let deferred = deferFulfillment(viewModel.actions) { $0.isContinueWithOIDC } context.send(viewAction: .confirm) try await deferred.fulfill() // Then a call to configure service should be made. XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) - XCTAssertNotEqual(service.homeserver.value.loginMode, .unknown) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptReceivedArguments?.prompt, .consent) + XCTAssertEqual(service.homeserver.value.loginMode, .oidc(supportsCreatePrompt: true)) } func testConfirmLoginAfterConfiguration() async throws { @@ -40,16 +44,19 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { XCTFail("The configuration should succeed.") return } - XCTAssertNotEqual(service.homeserver.value.loginMode, .unknown) + XCTAssertEqual(service.homeserver.value.loginMode, .oidc(supportsCreatePrompt: true)) XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 0) // When continuing from the confirmation screen. - let deferred = deferFulfillment(viewModel.actions) { $0 == .confirm } + let deferred = deferFulfillment(viewModel.actions) { $0.isContinueWithOIDC } context.send(viewAction: .confirm) try await deferred.fulfill() - // Then the configured homeserver should be used and no additional call should be made to the service. + // Then the configured homeserver should be used and no additional client should be built. XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptReceivedArguments?.prompt, .consent) } func testConfirmRegisterWithoutConfiguration() async throws { @@ -57,15 +64,19 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { setupViewModel(authenticationFlow: .register) XCTAssertEqual(service.homeserver.value.loginMode, .unknown) XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 0) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 0) // When continuing from the confirmation screen. - let deferred = deferFulfillment(viewModel.actions) { $0 == .confirm } + let deferred = deferFulfillment(viewModel.actions) { $0.isContinueWithOIDC } context.send(viewAction: .confirm) try await deferred.fulfill() // Then a call to configure service should be made. XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) - XCTAssertNotEqual(service.homeserver.value.loginMode, .unknown) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 1) + // The create prompt is broken: https://github.com/element-hq/matrix-authentication-service/issues/3429 + // XCTAssertEqual(client.urlForOidcOidcConfigurationPromptReceivedArguments?.prompt, .create) + XCTAssertEqual(service.homeserver.value.loginMode, .oidc(supportsCreatePrompt: true)) } func testConfirmRegisterAfterConfiguration() async throws { @@ -75,16 +86,59 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { XCTFail("The configuration should succeed.") return } - XCTAssertNotEqual(service.homeserver.value.loginMode, .unknown) + XCTAssertEqual(service.homeserver.value.loginMode, .oidc(supportsCreatePrompt: true)) XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 0) // When continuing from the confirmation screen. - let deferred = deferFulfillment(viewModel.actions) { $0 == .confirm } + let deferred = deferFulfillment(viewModel.actions) { $0.isContinueWithOIDC } context.send(viewAction: .confirm) try await deferred.fulfill() - // Then the configured homeserver should be used and no additional call should be made to the service. + // Then the configured homeserver should be used and no additional client should be built. XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) + // The create prompt is broken: https://github.com/element-hq/matrix-authentication-service/issues/3429 + // XCTAssertEqual(client.urlForOidcOidcConfigurationPromptReceivedArguments?.prompt, .create) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 1) + } + + func testConfirmPasswordLoginWithoutConfiguration() async throws { + // Given a view model for login using a service that hasn't been configured (against a server that doesn't support OIDC). + setupViewModel(authenticationFlow: .login, supportsOIDC: false) + XCTAssertEqual(service.homeserver.value.loginMode, .unknown) + XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 0) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 0) + + // When continuing from the confirmation screen. + let deferred = deferFulfillment(viewModel.actions) { $0.isContinueWithPassword } + context.send(viewAction: .confirm) + try await deferred.fulfill() + + // Then a call to configure service should be made, but not for the OIDC URL. + XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 0) + XCTAssertEqual(service.homeserver.value.loginMode, .password) + } + + func testConfirmPasswordLoginAfterConfiguration() async throws { + // Given a view model for login using a service that has already been configured (via the server selection screen). + setupViewModel(authenticationFlow: .login, supportsOIDC: false) + guard case .success = await service.configure(for: viewModel.state.homeserverAddress, flow: .login) else { + XCTFail("The configuration should succeed.") + return + } + XCTAssertEqual(service.homeserver.value.loginMode, .password) + XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 0) + + // When continuing from the confirmation screen. + let deferred = deferFulfillment(viewModel.actions) { $0.isContinueWithPassword } + context.send(viewAction: .confirm) + try await deferred.fulfill() + + // Then the configured homeserver should be used and no additional client should be built, nor a call to get the OIDC URL. + XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptCallsCount, 0) } func testRegistrationNotSupportedAlert() async throws { @@ -126,9 +180,9 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { private func setupViewModel(authenticationFlow: AuthenticationFlow, supportsOIDC: Bool = true, supportsOIDCCreatePrompt: Bool = true, supportsPasswordLogin: Bool = true) { // Manually create a configuration as the default homeserver address setting is immutable. - let client = ClientSDKMock(configuration: .init(oidcLoginURL: supportsOIDC ? "https://account.matrix.org/authorize" : nil, - supportsOIDCCreatePrompt: supportsOIDCCreatePrompt, - supportsPasswordLogin: supportsPasswordLogin)) + client = ClientSDKMock(configuration: .init(oidcLoginURL: supportsOIDC ? "https://account.matrix.org/authorize" : nil, + supportsOIDCCreatePrompt: supportsOIDCCreatePrompt, + supportsPasswordLogin: supportsPasswordLogin)) let configuration = AuthenticationClientBuilderMock.Configuration(homeserverClients: ["matrix.org": client], qrCodeClient: client) @@ -143,5 +197,24 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { authenticationFlow: authenticationFlow, slidingSyncLearnMoreURL: ServiceLocator.shared.settings.slidingSyncLearnMoreURL, userIndicatorController: UserIndicatorControllerMock()) + + // Add a fake window in order for the OIDC flow to continue + viewModel.context.send(viewAction: .updateWindow(UIWindow())) + } +} + +private extension ServerConfirmationScreenViewModelAction { + var isContinueWithOIDC: Bool { + switch self { + case .continueWithOIDC: true + default: false + } + } + + var isContinueWithPassword: Bool { + switch self { + case .continueWithPassword: true + default: false + } } }