From 6e561c89c39920c20b3a7d1741ec45341773b914 Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 20 May 2025 12:24:07 +0100 Subject: [PATCH 1/5] Add a picker mode to the ServerConfirmationScreen. --- .../en.lproj/Localizable.strings | 1 + .../AuthenticationFlowCoordinator.swift | 1 + ElementX/Sources/Generated/Strings.swift | 2 + .../Mocks/Generated/GeneratedMocks.swift | 2 +- .../ServerConfirmationScreenCoordinator.swift | 8 + .../ServerConfirmationScreenModels.swift | 34 ++-- .../ServerConfirmationScreenViewModel.swift | 61 ++++++-- .../View/ServerConfirmationScreen.swift | 105 +++++++++++-- .../Sources/GeneratedPreviewTests.swift | 2 +- ...verConfigurationScreenViewStateTests.swift | 16 +- ...rverConfirmationScreenViewModelTests.swift | 148 +++++++++++++++++- 11 files changed, 334 insertions(+), 46 deletions(-) diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings index ad5a026c35..a4768a07f3 100644 --- a/ElementX/Resources/Localizations/en.lproj/Localizable.strings +++ b/ElementX/Resources/Localizations/en.lproj/Localizable.strings @@ -993,6 +993,7 @@ "screen_server_confirmation_message_login_matrix_dot_org" = "Matrix is an open network for secure, decentralised communication."; "screen_server_confirmation_message_register" = "This is where your conversations will live — just like you would use an email provider to keep your emails."; "screen_server_confirmation_title_login" = "You’re about to sign in to %1$@"; +"screen_server_confirmation_title_picker_mode" = "Choose account provider"; "screen_server_confirmation_title_register" = "You’re about to create an account on %1$@"; "screen_session_verification_cancelled_subtitle" = "Something doesn’t seem right. Either the request timed out or the request was denied."; "screen_session_verification_compare_emojis_subtitle" = "Confirm that the emojis below match those shown on your other session."; diff --git a/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift index 749fd1e84c..0b39289e14 100644 --- a/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift @@ -341,6 +341,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { let parameters = ServerConfirmationScreenCoordinatorParameters(authenticationService: authenticationService, authenticationFlow: authenticationFlow, + appSettings: appSettings, userIndicatorController: userIndicatorController) let coordinator = ServerConfirmationScreenCoordinator(parameters: parameters) diff --git a/ElementX/Sources/Generated/Strings.swift b/ElementX/Sources/Generated/Strings.swift index e1f31f3c15..25a380d137 100644 --- a/ElementX/Sources/Generated/Strings.swift +++ b/ElementX/Sources/Generated/Strings.swift @@ -2458,6 +2458,8 @@ internal enum L10n { internal static func screenServerConfirmationTitleLogin(_ p1: Any) -> String { return L10n.tr("Localizable", "screen_server_confirmation_title_login", String(describing: p1)) } + /// Choose account provider + internal static var screenServerConfirmationTitlePickerMode: String { return L10n.tr("Localizable", "screen_server_confirmation_title_picker_mode") } /// You’re about to create an account on %1$@ internal static func screenServerConfirmationTitleRegister(_ p1: Any) -> String { return L10n.tr("Localizable", "screen_server_confirmation_title_register", String(describing: p1)) diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift index 92d457843c..23b6b23cba 100644 --- a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift +++ b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift @@ -1,4 +1,4 @@ -// Generated using Sourcery 2.2.6 — https://github.com/krzysztofzablocki/Sourcery +// Generated using Sourcery 2.2.7 — https://github.com/krzysztofzablocki/Sourcery // DO NOT EDIT // swiftlint:disable all diff --git a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenCoordinator.swift b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenCoordinator.swift index 55ad59d9b8..83c1efed1b 100644 --- a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenCoordinator.swift +++ b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenCoordinator.swift @@ -11,6 +11,7 @@ import SwiftUI struct ServerConfirmationScreenCoordinatorParameters { let authenticationService: AuthenticationServiceProtocol let authenticationFlow: AuthenticationFlow + let appSettings: AppSettings let userIndicatorController: UserIndicatorControllerProtocol } @@ -30,7 +31,14 @@ final class ServerConfirmationScreenCoordinator: CoordinatorProtocol { } init(parameters: ServerConfirmationScreenCoordinatorParameters) { + let mode = if parameters.appSettings.allowOtherAccountProviders { + ServerConfirmationScreenMode.confirmation(parameters.authenticationService.homeserver.value.address) + } else { + ServerConfirmationScreenMode.picker(parameters.appSettings.accountProviders) + } + viewModel = ServerConfirmationScreenViewModel(authenticationService: parameters.authenticationService, + mode: mode, authenticationFlow: parameters.authenticationFlow, userIndicatorController: parameters.userIndicatorController) } diff --git a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenModels.swift b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenModels.swift index 5ce1c80bf6..471ea6fef5 100644 --- a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenModels.swift +++ b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenModels.swift @@ -16,9 +16,16 @@ enum ServerConfirmationScreenViewModelAction { case changeServer } +enum ServerConfirmationScreenMode: Equatable { + /// The user is confirming the displayed account provider (or can enter their own). + case confirmation(String) + /// The user is only allowed to pick from a list of account providers. + case picker([String]) +} + struct ServerConfirmationScreenViewState: BindableState { - /// The homeserver address input by the user. - var homeserverAddress: String + /// Whether the screen is configured to confirm a single account provider or pick one from a list of many. + var mode: ServerConfirmationScreenMode /// The flow being attempted on the selected homeserver. let authenticationFlow: AuthenticationFlow /// The presentation anchor used for OIDC authentication. @@ -28,17 +35,24 @@ struct ServerConfirmationScreenViewState: BindableState { /// The screen's title. var title: String { - switch authenticationFlow { - case .login: - return L10n.screenServerConfirmationTitleLogin(homeserverAddress) - case .register: - return L10n.screenServerConfirmationTitleRegister(homeserverAddress) + switch mode { + case .confirmation(let accountProvider): + switch authenticationFlow { + case .login: + L10n.screenServerConfirmationTitleLogin(accountProvider) + case .register: + L10n.screenServerConfirmationTitleRegister(accountProvider) + } + case .picker: + L10n.screenServerConfirmationTitlePickerMode } } /// The message shown beneath the title. - var message: String { - switch authenticationFlow { + var message: String? { + guard case let .confirmation(homeserverAddress) = mode else { return nil } + + return switch authenticationFlow { case .login: if homeserverAddress == "matrix.org" { L10n.screenServerConfirmationMessageLoginMatrixDotOrg @@ -54,6 +68,8 @@ struct ServerConfirmationScreenViewState: BindableState { } struct ServerConfirmationScreenBindings { + /// The chosen server when in `.picker` mode, otherwise `nil`. + var pickerSelection: String? /// Information describing the currently displayed alert. var alertInfo: AlertInfo? } diff --git a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenViewModel.swift b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenViewModel.swift index be664bffb6..830f701c2c 100644 --- a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenViewModel.swift +++ b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/ServerConfirmationScreenViewModel.swift @@ -22,21 +22,29 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType, } init(authenticationService: AuthenticationServiceProtocol, + mode: ServerConfirmationScreenMode, authenticationFlow: AuthenticationFlow, userIndicatorController: UserIndicatorControllerProtocol) { self.authenticationService = authenticationService self.authenticationFlow = authenticationFlow self.userIndicatorController = userIndicatorController - let homeserver = authenticationService.homeserver.value - super.init(initialViewState: ServerConfirmationScreenViewState(homeserverAddress: homeserver.address, - authenticationFlow: authenticationFlow)) + let pickerSelection: String? = switch mode { + case .picker(let providers): providers[0] + case .confirmation: nil + } + + super.init(initialViewState: ServerConfirmationScreenViewState(mode: mode, + authenticationFlow: authenticationFlow, + bindings: .init(pickerSelection: pickerSelection))) - authenticationService.homeserver - .receive(on: DispatchQueue.main) - .map(\.address) - .weakAssign(to: \.state.homeserverAddress, on: self) - .store(in: &cancellables) + if case .confirmation = mode { + authenticationService.homeserver + .receive(on: DispatchQueue.main) + .map { .confirmation($0.address) } + .weakAssign(to: \.state.mode, on: self) + .store(in: &cancellables) + } } override func process(viewAction: ServerConfirmationScreenViewAction) { @@ -45,7 +53,10 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType, guard state.window != window else { return } Task { state.window = window } case .confirm: - Task { await confirmHomeserver() } + switch state.mode { + case .confirmation: Task { await confirmServer() } + case .picker: Task { await pickServer() } + } case .changeServer: actionsSubject.send(.changeServer) } @@ -53,16 +64,18 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType, // MARK: - Private - private func confirmHomeserver() async { + private func confirmServer() 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. + // If the login mode is unknown, the service hasn't been 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 { await fetchLoginURLIfNeededAndContinue() return } + // Note: We don't show the spinner until now as it isn't needed if the service is already + // configured and we're about to use password based login startLoading() defer { stopLoading() } @@ -87,6 +100,32 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType, } } + private func pickServer() async { + guard let accountProvider = state.bindings.pickerSelection else { + fatalError("It shouldn't be possible to confirm without a selection.") + } + + // Don't bother reconfiguring the service if it has already been done for the selected server. + let homeserver = authenticationService.homeserver.value + guard homeserver.loginMode == .unknown || homeserver.address != accountProvider else { + await fetchLoginURLIfNeededAndContinue() + return + } + + // Note: We don't show the spinner until now as it isn't needed if the service is already + // configured and we're about to use password based login + startLoading() + defer { stopLoading() } + + switch await authenticationService.configure(for: accountProvider, flow: authenticationFlow) { + case .success: + await fetchLoginURLIfNeededAndContinue() + case .failure: + // When the servers are hard-coded they should have a valid configuration, so show a generic error. + displayError(.unknownError) + } + } + private func fetchLoginURLIfNeededAndContinue() async { guard authenticationService.homeserver.value.loginMode.supportsOIDCFlow else { actionsSubject.send(.continueWithPassword) diff --git a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/View/ServerConfirmationScreen.swift b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/View/ServerConfirmationScreen.swift index 3e10e28f73..8815b4e738 100644 --- a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/View/ServerConfirmationScreen.swift +++ b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/View/ServerConfirmationScreen.swift @@ -11,14 +11,38 @@ import SwiftUI struct ServerConfirmationScreen: View { @Bindable var context: ServerConfirmationScreenViewModel.Context + private var backgroundColor: Color { + switch context.viewState.mode { + case .confirmation: .compound.bgCanvasDefault + case .picker: .compound.bgSubtleSecondaryLevel0 + } + } + + private var headerIcon: KeyPath { + switch context.viewState.mode { + case .confirmation: \.userProfileSolid + case .picker: \.homeSolid + } + } + + private var headerIconStyle: BigIcon.Style { + switch context.viewState.mode { + case .confirmation: .defaultSolid + case .picker: .default + } + } + var body: some View { FullscreenDialog(topPadding: UIConstants.iconTopPaddingToNavigationBar) { - header + VStack(spacing: 36) { + header + mainContent + } } bottomContent: { buttons } .background() - .backgroundStyle(.compound.bgCanvasDefault) + .backgroundStyle(backgroundColor) .alert(item: $context.alertInfo) .introspect(.window, on: .supportedVersions) { window in context.send(viewAction: .updateWindow(window)) @@ -28,8 +52,7 @@ struct ServerConfirmationScreen: View { /// The main content of the view to be shown in a scroll view. var header: some View { VStack(spacing: 8) { - Image(systemSymbol: .personCropCircleFill) - .bigIcon() + BigIcon(icon: headerIcon, style: headerIconStyle) .padding(.bottom, 8) Text(context.viewState.title) @@ -38,14 +61,25 @@ struct ServerConfirmationScreen: View { .foregroundColor(.compound.textPrimary) .fixedSize(horizontal: false, vertical: true) - Text(context.viewState.message) - .font(.compound.bodyMD) - .multilineTextAlignment(.center) - .foregroundColor(.compound.textSecondary) + if let message = context.viewState.message { + Text(message) + .font(.compound.bodyMD) + .multilineTextAlignment(.center) + .foregroundColor(.compound.textSecondary) + } } .padding(.horizontal, 16) } + @ViewBuilder + var mainContent: some View { + if case .picker(let accountProviders) = context.viewState.mode { + FakeInlinePicker(items: accountProviders, + icon: \.host, + selection: $context.pickerSelection) + } + } + /// The action buttons shown at the bottom of the view. var buttons: some View { VStack(spacing: 16) { @@ -55,21 +89,53 @@ struct ServerConfirmationScreen: View { .buttonStyle(.compound(.primary)) .accessibilityIdentifier(A11yIdentifiers.serverConfirmationScreen.continue) - Button { context.send(viewAction: .changeServer) } label: { - Text(L10n.screenServerConfirmationChangeServer) - .font(.compound.bodyLGSemibold) - .padding(14) + if case .confirmation = context.viewState.mode { + Button { context.send(viewAction: .changeServer) } label: { + Text(L10n.screenServerConfirmationChangeServer) + .font(.compound.bodyLGSemibold) + .padding(14) + } + .accessibilityIdentifier(A11yIdentifiers.serverConfirmationScreen.changeServer) } - .accessibilityIdentifier(A11yIdentifiers.serverConfirmationScreen.changeServer) } } } +// This is such a hack. I hate it! +// But… We're not in a List/Form, the compound picker doesn't +// support icons and this screen's design might change so 🤷‍♂️. +private struct FakeInlinePicker: View { + let items: [String] + let icon: KeyPath + @Binding var selection: String? + + var body: some View { + VStack(spacing: 0) { + ForEach(items, id: \.self) { item in + ListRow(label: .default(title: item, icon: icon), + kind: .selection(isSelected: selection == item) { + selection = item + }) + .overlay(alignment: .bottom) { + if item != items.last { + Divider() + .hidden() + .overlay(Color.compound._borderInteractiveSecondaryAlpha) + .padding(.leading, 54) + } + } + } + } + .clipShape(RoundedRectangle(cornerRadius: 10)) + } +} + // MARK: - Previews struct ServerConfirmationScreen_Previews: PreviewProvider, TestablePreview { - static let loginViewModel = makeViewModel(flow: .login) - static let registerViewModel = makeViewModel(flow: .register) + static let loginViewModel = makeViewModel(mode: .confirmation("matrix.org"), flow: .login) + static let registerViewModel = makeViewModel(mode: .confirmation("matrix.org"), flow: .register) + static let pickerViewModel = makeViewModel(mode: .picker(["dept1.company.com", "dept2.company.com", "dept3.company.com"]), flow: .login) static var previews: some View { NavigationStack { @@ -83,10 +149,17 @@ struct ServerConfirmationScreen_Previews: PreviewProvider, TestablePreview { .toolbar(.visible, for: .navigationBar) } .previewDisplayName("Register") + + NavigationStack { + ServerConfirmationScreen(context: pickerViewModel.context) + .toolbar(.visible, for: .navigationBar) + } + .previewDisplayName("Picker") } - static func makeViewModel(flow: AuthenticationFlow) -> ServerConfirmationScreenViewModel { + static func makeViewModel(mode: ServerConfirmationScreenMode, flow: AuthenticationFlow) -> ServerConfirmationScreenViewModel { ServerConfirmationScreenViewModel(authenticationService: AuthenticationService.mock, + mode: mode, authenticationFlow: flow, userIndicatorController: UserIndicatorControllerMock()) } diff --git a/PreviewTests/Sources/GeneratedPreviewTests.swift b/PreviewTests/Sources/GeneratedPreviewTests.swift index 03a622860b..0f3b43ce1f 100644 --- a/PreviewTests/Sources/GeneratedPreviewTests.swift +++ b/PreviewTests/Sources/GeneratedPreviewTests.swift @@ -1,4 +1,4 @@ -// Generated using Sourcery 2.2.6 — https://github.com/krzysztofzablocki/Sourcery +// Generated using Sourcery 2.2.7 — https://github.com/krzysztofzablocki/Sourcery // DO NOT EDIT // swiftlint:disable all diff --git a/UnitTests/Sources/ServerConfigurationScreenViewStateTests.swift b/UnitTests/Sources/ServerConfigurationScreenViewStateTests.swift index 90a92efdb7..66fc72874b 100644 --- a/UnitTests/Sources/ServerConfigurationScreenViewStateTests.swift +++ b/UnitTests/Sources/ServerConfigurationScreenViewStateTests.swift @@ -12,25 +12,29 @@ import XCTest @MainActor class ServerConfirmationScreenViewStateTests: XCTestCase { func testLoginMessageString() { - let matrixDotOrgLogin = ServerConfirmationScreenViewState(homeserverAddress: LoginHomeserver.mockMatrixDotOrg.address, + let matrixDotOrgLogin = ServerConfirmationScreenViewState(mode: .confirmation(LoginHomeserver.mockMatrixDotOrg.address), authenticationFlow: .login) XCTAssertEqual(matrixDotOrgLogin.message, L10n.screenServerConfirmationMessageLoginMatrixDotOrg, "matrix.org should have a custom message.") - let elementDotIoLogin = ServerConfirmationScreenViewState(homeserverAddress: "element.io", + let elementDotIoLogin = ServerConfirmationScreenViewState(mode: .confirmation("element.io"), authenticationFlow: .login) XCTAssertEqual(elementDotIoLogin.message, L10n.screenServerConfirmationMessageLoginElementDotIo, "element.io should have a custom message.") - let otherLogin = ServerConfirmationScreenViewState(homeserverAddress: LoginHomeserver.mockOIDC.address, + let otherLogin = ServerConfirmationScreenViewState(mode: .confirmation(LoginHomeserver.mockOIDC.address), authenticationFlow: .login) - XCTAssertTrue(otherLogin.message.isEmpty, "Other servers should not show a message.") + XCTAssertEqual(otherLogin.message, "", "Other servers should not show a message.") + + let pickerLogin = ServerConfirmationScreenViewState(mode: .picker(["element.io", "matrix.org"]), + authenticationFlow: .login) + XCTAssertNil(pickerLogin.message, "The picker mode should not show a message.") } func testRegisterMessageString() { - let matrixDotOrgRegister = ServerConfirmationScreenViewState(homeserverAddress: LoginHomeserver.mockMatrixDotOrg.address, + let matrixDotOrgRegister = ServerConfirmationScreenViewState(mode: .confirmation(LoginHomeserver.mockMatrixDotOrg.address), authenticationFlow: .register) XCTAssertEqual(matrixDotOrgRegister.message, L10n.screenServerConfirmationMessageRegister, "The registration message should always be the same.") - let oidcRegister = ServerConfirmationScreenViewState(homeserverAddress: LoginHomeserver.mockOIDC.address, + let oidcRegister = ServerConfirmationScreenViewState(mode: .confirmation(LoginHomeserver.mockOIDC.address), authenticationFlow: .register) XCTAssertEqual(oidcRegister.message, L10n.screenServerConfirmationMessageRegister, "The registration message should always be the same.") } diff --git a/UnitTests/Sources/ServerConfirmationScreenViewModelTests.swift b/UnitTests/Sources/ServerConfirmationScreenViewModelTests.swift index 7ae3dea326..ca66de98b8 100644 --- a/UnitTests/Sources/ServerConfirmationScreenViewModelTests.swift +++ b/UnitTests/Sources/ServerConfirmationScreenViewModelTests.swift @@ -14,14 +14,28 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { var clientBuilderFactory: AuthenticationClientBuilderFactoryMock! var client: ClientSDKMock! var service: AuthenticationServiceProtocol! + var appSettings: AppSettings! var viewModel: ServerConfirmationScreenViewModel! var context: ServerConfirmationScreenViewModel.Context { viewModel.context } + override func setUp() { + AppSettings.resetAllSettings() + appSettings = AppSettings() + ServiceLocator.shared.register(appSettings: appSettings) + } + + override func tearDown() { + AppSettings.resetAllSettings() + } + + // MARK: - Confirmation mode + func testConfirmLoginWithoutConfiguration() async throws { // Given a view model for login using a service that hasn't been configured. setupViewModel(authenticationFlow: .login) XCTAssertEqual(service.homeserver.value.loginMode, .unknown) + XCTAssertEqual(context.viewState.mode, .confirmation(service.homeserver.value.address)) XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 0) XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 0) @@ -45,6 +59,7 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { return } XCTAssertEqual(service.homeserver.value.loginMode, .oidc(supportsCreatePrompt: true)) + XCTAssertEqual(context.viewState.mode, .confirmation(service.homeserver.value.address)) XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 0) @@ -63,6 +78,7 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { // Given a view model for registration using a service that hasn't been configured. setupViewModel(authenticationFlow: .register) XCTAssertEqual(service.homeserver.value.loginMode, .unknown) + XCTAssertEqual(context.viewState.mode, .confirmation(service.homeserver.value.address)) XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 0) XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 0) @@ -87,6 +103,7 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { return } XCTAssertEqual(service.homeserver.value.loginMode, .oidc(supportsCreatePrompt: true)) + XCTAssertEqual(context.viewState.mode, .confirmation(service.homeserver.value.address)) XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 0) @@ -106,6 +123,7 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { // 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(context.viewState.mode, .confirmation(service.homeserver.value.address)) XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 0) XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 0) @@ -128,6 +146,7 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { return } XCTAssertEqual(service.homeserver.value.loginMode, .password) + XCTAssertEqual(context.viewState.mode, .confirmation(service.homeserver.value.address)) XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 0) @@ -176,9 +195,122 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { XCTAssertEqual(context.alertInfo?.id, .login) } + // MARK: - Picker mode + + func testPickerWithoutConfiguration() async throws { + // Given a view model for login using a service that hasn't been configured. + setupViewModel(authenticationFlow: .login, restrictedFlow: true) + XCTAssertEqual(service.homeserver.value.loginMode, .unknown) + XCTAssertEqual(context.viewState.mode, .picker(appSettings.accountProviders)) + XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 0) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 0) + + // When continuing from the confirmation screen. + 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) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintReceivedArguments?.prompt, .consent) + XCTAssertEqual(service.homeserver.value.loginMode, .oidc(supportsCreatePrompt: true)) + } + + func testPickerAfterConfiguration() async throws { + // Given a view model for login using a service that has already been configured (via the server selection screen). + setupViewModel(authenticationFlow: .login, restrictedFlow: true) + guard case .success = await service.configure(for: appSettings.accountProviders[0], flow: .login) else { + XCTFail("The configuration should succeed.") + return + } + XCTAssertEqual(service.homeserver.value.loginMode, .oidc(supportsCreatePrompt: true)) + XCTAssertEqual(context.viewState.mode, .picker(appSettings.accountProviders)) + XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 0) + + // When continuing from the confirmation screen. + 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 client should be built. + XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintReceivedArguments?.prompt, .consent) + } + + func testPickerForPasswordLoginWithoutConfiguration() 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, restrictedFlow: true) + XCTAssertEqual(service.homeserver.value.loginMode, .unknown) + XCTAssertEqual(context.viewState.mode, .picker(appSettings.accountProviders)) + XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 0) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 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.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 0) + XCTAssertEqual(service.homeserver.value.loginMode, .password) + } + + func testPickerForPasswordLoginAfterConfiguration() 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, restrictedFlow: true) + guard case .success = await service.configure(for: appSettings.accountProviders[0], flow: .login) else { + XCTFail("The configuration should succeed.") + return + } + XCTAssertEqual(service.homeserver.value.loginMode, .password) + XCTAssertEqual(context.viewState.mode, .picker(appSettings.accountProviders)) + XCTAssertEqual(clientBuilderFactory.makeBuilderSessionDirectoriesPassphraseClientSessionDelegateAppSettingsAppHooksCallsCount, 1) + XCTAssertEqual(client.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 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.urlForOidcOidcConfigurationPromptLoginHintCallsCount, 0) + } + // MARK: - Helpers - private func setupViewModel(authenticationFlow: AuthenticationFlow, supportsOIDC: Bool = true, supportsOIDCCreatePrompt: Bool = true, supportsPasswordLogin: Bool = true) { + private func setupViewModel(authenticationFlow: AuthenticationFlow, + supportsOIDC: Bool = true, + supportsOIDCCreatePrompt: Bool = true, + supportsPasswordLogin: Bool = true, + restrictedFlow: Bool = false) { + var mode = ServerConfirmationScreenMode.confirmation("matrix.org") + if restrictedFlow { + appSettings.override(accountProviders: ["matrix.org", "beta.matrix.org"], + allowOtherAccountProviders: false, + pushGatewayBaseURL: appSettings.pushGatewayBaseURL, + oidcRedirectURL: appSettings.oidcRedirectURL, + websiteURL: appSettings.websiteURL, + logoURL: appSettings.logoURL, + copyrightURL: appSettings.copyrightURL, + acceptableUseURL: appSettings.acceptableUseURL, + privacyURL: appSettings.privacyURL, + encryptionURL: appSettings.encryptionURL, + deviceVerificationURL: appSettings.deviceVerificationURL, + chatBackupDetailsURL: appSettings.chatBackupDetailsURL, + identityPinningViolationDetailsURL: appSettings.identityPinningViolationDetailsURL, + elementWebHosts: appSettings.elementWebHosts, + accountProvisioningHost: appSettings.accountProvisioningHost, + bugReportApplicationID: appSettings.bugReportApplicationID, + analyticsTermsURL: appSettings.analyticsTermsURL, + mapTilerConfiguration: appSettings.mapTilerConfiguration) + mode = .picker(appSettings.accountProviders) + } + // Manually create a configuration as the default homeserver address setting is immutable. client = ClientSDKMock(configuration: .init(oidcLoginURL: supportsOIDC ? "https://account.matrix.org/authorize" : nil, supportsOIDCCreatePrompt: supportsOIDCCreatePrompt, @@ -190,10 +322,11 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { service = AuthenticationService(userSessionStore: UserSessionStoreMock(configuration: .init()), encryptionKeyProvider: EncryptionKeyProvider(), clientBuilderFactory: clientBuilderFactory, - appSettings: ServiceLocator.shared.settings, + appSettings: appSettings, appHooks: AppHooks()) viewModel = ServerConfirmationScreenViewModel(authenticationService: service, + mode: mode, authenticationFlow: authenticationFlow, userIndicatorController: UserIndicatorControllerMock()) @@ -202,6 +335,17 @@ class ServerConfirmationScreenViewModelTests: XCTestCase { } } +private extension ServerConfirmationScreenViewState { + var homeserverAddress: String { + switch mode { + case .confirmation(let accountProvider): + accountProvider + case .picker: + fatalError() + } + } +} + private extension ServerConfirmationScreenViewModelAction { var isContinueWithOIDC: Bool { switch self { From db130e2e77bb2e6cdb619abfcd63482c054abc3d Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 20 May 2025 12:52:57 +0100 Subject: [PATCH 2/5] Hook up the account provider picker in authentication the flow. Simplify the authentication flow coordinator, removing the restricted state. --- .../AuthenticationFlowCoordinator.swift | 65 +++++++++---------- .../AuthenticationStartScreenViewModel.swift | 10 +-- 2 files changed, 33 insertions(+), 42 deletions(-) diff --git a/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift index 0b39289e14..242c359bcd 100644 --- a/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/AuthenticationFlowCoordinator.swift @@ -31,8 +31,6 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { /// The initial screen shown when you first launch the app. case startScreen - /// The initial screen with the selection of account provider having been restricted. - case restrictedStartScreen /// The screen used for the whole QR Code flow. case qrCodeLoginScreen @@ -55,7 +53,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { enum Event: EventType { /// The flow is being started. - case start(allowOtherAccountProviders: Bool) + case start /// Modify the flow using the provisioning parameters in the `userInfo`. case applyProvisioningParameters @@ -68,7 +66,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { case reportProblem /// The QR login flow was aborted. - case cancelledLoginWithQR(previousState: State) + case cancelledLoginWithQR /// The user aborted manual login. case cancelledServerConfirmation @@ -87,7 +85,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { case cancelledPasswordLogin(previousState: State) /// The user has finished reporting a problem (or viewing the logs). - case bugReportFlowComplete(previousState: State) + case bugReportFlowComplete /// The user has successfully signed in. The new session can be found in the `userInfo`. case signedIn @@ -127,7 +125,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { } func start() { - stateMachine.tryEvent(.start(allowOtherAccountProviders: appSettings.allowOtherAccountProviders)) + stateMachine.tryEvent(.start) } func handleAppRoute(_ appRoute: AppRoute, animated: Bool) { @@ -150,11 +148,11 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { func clearRoute(animated: Bool) { switch stateMachine.state { - case .initial, .startScreen, .restrictedStartScreen: + case .initial, .startScreen: break case .qrCodeLoginScreen: navigationStackCoordinator.setSheetCoordinator(nil) - stateMachine.tryEvent(.cancelledLoginWithQR(previousState: .initial)) // Needs to be handled manually. + stateMachine.tryEvent(.cancelledLoginWithQR) // Needs to be handled manually. case .serverConfirmationScreen: navigationStackCoordinator.popToRoot(animated: animated) case .serverSelectionScreen: @@ -175,27 +173,22 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { // MARK: - Setup private func configureStateMachine() { - stateMachine.addRoutes(event: .start(allowOtherAccountProviders: true), transitions: [.initial => .startScreen]) { [weak self] _ in - self?.showStartScreen(fromState: .initial) - } - stateMachine.addRoutes(event: .start(allowOtherAccountProviders: false), transitions: [.initial => .restrictedStartScreen]) { [weak self] _ in + stateMachine.addRoutes(event: .start, transitions: [.initial => .startScreen]) { [weak self] _ in self?.showStartScreen(fromState: .initial) } - stateMachine.addRoutes(event: .applyProvisioningParameters, transitions: [.initial => .restrictedStartScreen, - .startScreen => .restrictedStartScreen]) { [weak self] context in + stateMachine.addRoutes(event: .applyProvisioningParameters, transitions: [.initial => .startScreen, + .startScreen => .startScreen]) { [weak self] context in guard let provisioningParameters = context.userInfo as? AccountProvisioningParameters else { fatalError("The authentication configuration is missing.") } self?.showStartScreen(fromState: context.fromState, applying: provisioningParameters) } // QR Code - stateMachine.addRoutes(event: .loginWithQR, transitions: [.startScreen => .qrCodeLoginScreen, - .restrictedStartScreen => .qrCodeLoginScreen]) { [weak self] context in - self?.showQRCodeLoginScreen(fromState: context.fromState) + stateMachine.addRoutes(event: .loginWithQR, transitions: [.startScreen => .qrCodeLoginScreen]) { [weak self] _ in + self?.showQRCodeLoginScreen() } - stateMachine.addRoutes(event: .cancelledLoginWithQR(previousState: .startScreen), transitions: [.qrCodeLoginScreen => .startScreen]) - stateMachine.addRoutes(event: .cancelledLoginWithQR(previousState: .restrictedStartScreen), transitions: [.qrCodeLoginScreen => .restrictedStartScreen]) + stateMachine.addRoutes(event: .cancelledLoginWithQR, transitions: [.qrCodeLoginScreen => .startScreen]) // Manual Authentication @@ -216,31 +209,29 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { stateMachine.addRoutes(event: .dismissedServerSelection, transitions: [.serverSelectionScreen => .serverConfirmationScreen]) stateMachine.addRoutes(event: .continueWithOIDC, transitions: [.serverConfirmationScreen => .oidcAuthentication, - .restrictedStartScreen => .oidcAuthentication]) { [weak self] context in + .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: .restrictedStartScreen), transitions: [.oidcAuthentication => .restrictedStartScreen]) + stateMachine.addRoutes(event: .cancelledOIDCAuthentication(previousState: .startScreen), transitions: [.oidcAuthentication => .startScreen]) stateMachine.addRoutes(event: .continueWithPassword, transitions: [.serverConfirmationScreen => .loginScreen, - .restrictedStartScreen => .loginScreen]) { [weak self] context in + .startScreen => .loginScreen]) { [weak self] context in let loginHint = context.userInfo as? String self?.showLoginScreen(loginHint: loginHint, fromState: context.fromState) } stateMachine.addRoutes(event: .cancelledPasswordLogin(previousState: .serverConfirmationScreen), transitions: [.loginScreen => .serverConfirmationScreen]) - stateMachine.addRoutes(event: .cancelledPasswordLogin(previousState: .restrictedStartScreen), transitions: [.loginScreen => .restrictedStartScreen]) + stateMachine.addRoutes(event: .cancelledPasswordLogin(previousState: .startScreen), transitions: [.loginScreen => .startScreen]) // Bug Report - stateMachine.addRoutes(event: .reportProblem, transitions: [.startScreen => .bugReportFlow, - .restrictedStartScreen => .bugReportFlow]) { [weak self] context in - self?.startBugReportFlow(fromState: context.fromState) + stateMachine.addRoutes(event: .reportProblem, transitions: [.startScreen => .bugReportFlow]) { [weak self] _ in + self?.startBugReportFlow() } - stateMachine.addRoutes(event: .bugReportFlowComplete(previousState: .startScreen), transitions: [.bugReportFlow => .startScreen]) - stateMachine.addRoutes(event: .bugReportFlowComplete(previousState: .restrictedStartScreen), transitions: [.bugReportFlow => .restrictedStartScreen]) + stateMachine.addRoutes(event: .bugReportFlowComplete, transitions: [.bugReportFlow => .startScreen]) // Completion @@ -251,6 +242,12 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { self?.userHasSignedIn(userSession: userSession) } + // Logging + + stateMachine.addAnyHandler(.any => .any) { context in + MXLog.dev("Transitioning from `\(context.fromState)` to `\(context.toState)` with event `\(String(describing: context.event))`.") + } + // Unhandled stateMachine.addErrorHandler { context in @@ -302,9 +299,9 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { // MARK: - QR Code - private func showQRCodeLoginScreen(fromState: State) { + private func showQRCodeLoginScreen() { let coordinator = QRCodeLoginScreenCoordinator(parameters: .init(qrCodeLoginService: qrCodeLoginService, - canSignInManually: fromState != .restrictedStartScreen, + canSignInManually: appSettings.allowOtherAccountProviders, // No need to worry about provisioning links as we hide QR login. orientationManager: appMediator.windowManager, appMediator: appMediator)) coordinator.actionsPublisher.sink { [weak self] action in @@ -314,11 +311,11 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { switch action { case .signInManually: navigationStackCoordinator.setSheetCoordinator(nil) - stateMachine.tryEvent(.cancelledLoginWithQR(previousState: fromState)) + stateMachine.tryEvent(.cancelledLoginWithQR) stateMachine.tryEvent(.confirmServer(.login)) case .cancel: navigationStackCoordinator.setSheetCoordinator(nil) - stateMachine.tryEvent(.cancelledLoginWithQR(previousState: fromState)) + stateMachine.tryEvent(.cancelledLoginWithQR) case .done(let userSession): navigationStackCoordinator.setSheetCoordinator(nil) // Since the qr code login flow includes verification @@ -438,7 +435,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { // MARK: - Bug Report - private func startBugReportFlow(fromState: State) { + private func startBugReportFlow() { let coordinator = BugReportFlowCoordinator(parameters: .init(presentationMode: .sheet(navigationStackCoordinator), userIndicatorController: userIndicatorController, bugReportService: bugReportService, @@ -446,7 +443,7 @@ class AuthenticationFlowCoordinator: FlowCoordinatorProtocol { coordinator.actionsPublisher.sink { [weak self] action in switch action { case .complete: - self?.stateMachine.tryEvent(.bugReportFlowComplete(previousState: fromState)) + self?.stateMachine.tryEvent(.bugReportFlowComplete) } } .store(in: &cancellables) diff --git a/ElementX/Sources/Screens/AuthenticationStartScreen/AuthenticationStartScreenViewModel.swift b/ElementX/Sources/Screens/AuthenticationStartScreen/AuthenticationStartScreenViewModel.swift index e58bb4487f..fa0c7e05c2 100644 --- a/ElementX/Sources/Screens/AuthenticationStartScreen/AuthenticationStartScreenViewModel.swift +++ b/ElementX/Sources/Screens/AuthenticationStartScreen/AuthenticationStartScreenViewModel.swift @@ -77,14 +77,8 @@ class AuthenticationStartScreenViewModel: AuthenticationStartScreenViewModelType // MARK: - Private private func login() async { - if !appSettings.allowOtherAccountProviders { - if appSettings.accountProviders.count == 1 { - await configureAccountProvider(appSettings.accountProviders[0]) - } else { - fatalError("WIP: Account provider picker not implemented.") - } - } else if let provisioningParameters { - await configureAccountProvider(provisioningParameters.accountProvider, loginHint: provisioningParameters.loginHint) + if let serverName = state.serverName { + await configureAccountProvider(serverName, loginHint: provisioningParameters?.loginHint) } else { actionsSubject.send(.login) // No need to configure anything here, continue the flow. } From 9d88115e3f103faa57609a4d2880ab498b1d0f0b Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 20 May 2025 15:39:59 +0100 Subject: [PATCH 3/5] UI/Snapshot tests. --- .../Other/AccessibilityIdentifiers.swift | 1 + .../View/ServerConfirmationScreen.swift | 1 + .../UITests/UITestsAppCoordinator.swift | 7 ++--- .../UITests/UITestsScreenIdentifier.swift | 1 + ...erConfirmationScreen.Picker-iPad-en-GB.png | 3 +++ ...rConfirmationScreen.Picker-iPad-pseudo.png | 3 +++ ...firmationScreen.Picker-iPhone-16-en-GB.png | 3 +++ ...irmationScreen.Picker-iPhone-16-pseudo.png | 3 +++ .../AuthenticationFlowCoordinatorTests.swift | 26 +++++++++++++++++++ ...idersLoginWithPassword-iPad-18-4-en-GB.png | 3 +++ ...ersLoginWithPassword-iPhone-18-4-en-GB.png | 3 +++ 11 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPad-en-GB.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPad-pseudo.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPhone-16-en-GB.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPhone-16-pseudo.png create mode 100644 UITests/Sources/__Snapshots__/Application/authenticationFlowCoordinator.testMultipleProvidersLoginWithPassword-iPad-18-4-en-GB.png create mode 100644 UITests/Sources/__Snapshots__/Application/authenticationFlowCoordinator.testMultipleProvidersLoginWithPassword-iPhone-18-4-en-GB.png diff --git a/ElementX/Sources/Other/AccessibilityIdentifiers.swift b/ElementX/Sources/Other/AccessibilityIdentifiers.swift index 7ab58719be..439f54affc 100644 --- a/ElementX/Sources/Other/AccessibilityIdentifiers.swift +++ b/ElementX/Sources/Other/AccessibilityIdentifiers.swift @@ -214,6 +214,7 @@ enum A11yIdentifiers { struct ServerConfirmationScreen { let `continue` = "server_confirmation-continue" let changeServer = "server_confirmation-change_server" + let serverPicker = "server_confirmation-server_picker" } struct SessionVerificationScreen { diff --git a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/View/ServerConfirmationScreen.swift b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/View/ServerConfirmationScreen.swift index 8815b4e738..caa3a605af 100644 --- a/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/View/ServerConfirmationScreen.swift +++ b/ElementX/Sources/Screens/Authentication/ServerConfirmationScreen/View/ServerConfirmationScreen.swift @@ -77,6 +77,7 @@ struct ServerConfirmationScreen: View { FakeInlinePicker(items: accountProviders, icon: \.host, selection: $context.pickerSelection) + .accessibilityIdentifier(A11yIdentifiers.serverConfirmationScreen.serverPicker) } } diff --git a/ElementX/Sources/UITests/UITestsAppCoordinator.swift b/ElementX/Sources/UITests/UITestsAppCoordinator.swift index 371d4dd4fe..940f18826c 100644 --- a/ElementX/Sources/UITests/UITestsAppCoordinator.swift +++ b/ElementX/Sources/UITests/UITestsAppCoordinator.swift @@ -120,11 +120,12 @@ class MockScreen: Identifiable { userIndicatorController: ServiceLocator.shared.userIndicatorController)) navigationStackCoordinator.setRootCoordinator(coordinator) return navigationStackCoordinator - case .authenticationFlow, .provisionedAuthenticationFlow, .singleProviderAuthenticationFlow: + case .authenticationFlow, .provisionedAuthenticationFlow, .singleProviderAuthenticationFlow, .multipleProvidersAuthenticationFlow: let appSettings: AppSettings! = ServiceLocator.shared.settings - if id == .singleProviderAuthenticationFlow { - appSettings.override(accountProviders: ["example.com"], + if id == .singleProviderAuthenticationFlow || id == .multipleProvidersAuthenticationFlow { + let accountProviders = id == .singleProviderAuthenticationFlow ? ["example.com"] : ["guest.example.com", "example.com"] + appSettings.override(accountProviders: accountProviders, allowOtherAccountProviders: false, pushGatewayBaseURL: appSettings.pushGatewayBaseURL, oidcRedirectURL: appSettings.oidcRedirectURL, diff --git a/ElementX/Sources/UITests/UITestsScreenIdentifier.swift b/ElementX/Sources/UITests/UITestsScreenIdentifier.swift index fcc6921f22..f0e3830921 100644 --- a/ElementX/Sources/UITests/UITestsScreenIdentifier.swift +++ b/ElementX/Sources/UITests/UITestsScreenIdentifier.swift @@ -18,6 +18,7 @@ enum UITestsScreenIdentifier: String { case authenticationFlow case provisionedAuthenticationFlow case singleProviderAuthenticationFlow + case multipleProvidersAuthenticationFlow case bugReport case createPoll case createRoom diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPad-en-GB.png new file mode 100644 index 0000000000..e9f01ce9b2 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPad-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c31aa1aecf3ce750afab447cf6944cf45fb3be5f7f46d316565871d0e7b64bfe +size 115397 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPad-pseudo.png new file mode 100644 index 0000000000..b1310df91b --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPad-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0e3ceffe60f0d57d8fa2e4293a5c3de9dca2590b7c0a7a0ab4066e52ae17ccfc +size 117262 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPhone-16-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPhone-16-en-GB.png new file mode 100644 index 0000000000..97f552db47 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPhone-16-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b3413e63a5e7c2ecea6519851f1cd4d71a9bcfe04ca305b73dea61cdad00dc99 +size 63440 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPhone-16-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPhone-16-pseudo.png new file mode 100644 index 0000000000..11fab8a42d --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Picker-iPhone-16-pseudo.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c3a3c13ef7d79d777eab6e6c595937340ab626eaa40b62e071a6996249a62eba +size 71974 diff --git a/UITests/Sources/AuthenticationFlowCoordinatorTests.swift b/UITests/Sources/AuthenticationFlowCoordinatorTests.swift index 7afec37688..289fc2a8a9 100644 --- a/UITests/Sources/AuthenticationFlowCoordinatorTests.swift +++ b/UITests/Sources/AuthenticationFlowCoordinatorTests.swift @@ -202,6 +202,32 @@ class AuthenticationFlowCoordinatorUITests: XCTestCase { app.buttons[A11yIdentifiers.loginScreen.continue].tap() } + func testMultipleProvidersLoginWithPassword() async throws { + // Given the authentication flow with only 2 allowed servers. + let app = Application.launch(.multipleProvidersAuthenticationFlow) + + // Then the start screen should be configured appropriately. + try await app.assertScreenshot() + + // Splash Screen: Tap get started button + app.buttons[A11yIdentifiers.authenticationStartScreen.signIn].tap() + + // Server Confirmation: Tap the picker and confirm + app.switches.matching(identifier: A11yIdentifiers.serverConfirmationScreen.serverPicker).element(boundBy: 1).tap() + app.buttons[A11yIdentifiers.serverConfirmationScreen.continue].tap() + + // Login Screen: Wait for continue button to appear + let continueButton = app.buttons[A11yIdentifiers.loginScreen.continue] + XCTAssertTrue(continueButton.waitForExistence(timeout: 2.0)) + + // Login Screen: Enter valid credentials + app.textFields[A11yIdentifiers.loginScreen.emailUsername].clearAndTypeText("alice\n", app: app) + app.secureTextFields[A11yIdentifiers.loginScreen.password].clearAndTypeText("12345678", app: app) + + // Login Screen: Tap next + app.buttons[A11yIdentifiers.loginScreen.continue].tap() + } + func verifyReportBugButton(_ app: XCUIApplication) async throws { // Splash Screen: Report a problem button. app.buttons[A11yIdentifiers.authenticationStartScreen.reportAProblem].tap() diff --git a/UITests/Sources/__Snapshots__/Application/authenticationFlowCoordinator.testMultipleProvidersLoginWithPassword-iPad-18-4-en-GB.png b/UITests/Sources/__Snapshots__/Application/authenticationFlowCoordinator.testMultipleProvidersLoginWithPassword-iPad-18-4-en-GB.png new file mode 100644 index 0000000000..e17f15cdb8 --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/authenticationFlowCoordinator.testMultipleProvidersLoginWithPassword-iPad-18-4-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:80fd069026bfa66edfa0dc3504e600e5d32fc1b081f255cc9c7dae7643bd3610 +size 1335111 diff --git a/UITests/Sources/__Snapshots__/Application/authenticationFlowCoordinator.testMultipleProvidersLoginWithPassword-iPhone-18-4-en-GB.png b/UITests/Sources/__Snapshots__/Application/authenticationFlowCoordinator.testMultipleProvidersLoginWithPassword-iPhone-18-4-en-GB.png new file mode 100644 index 0000000000..2382b2a76e --- /dev/null +++ b/UITests/Sources/__Snapshots__/Application/authenticationFlowCoordinator.testMultipleProvidersLoginWithPassword-iPhone-18-4-en-GB.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c0f65ff050c24acbff22f9687103ea0fe62bd50c20a9af97b5413393697fa173 +size 1207677 From 6f9fcb1d52f0768f804977e2440073aff3cb0a58 Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 20 May 2025 15:40:14 +0100 Subject: [PATCH 4/5] Package.resolved --- Package.resolved | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Package.resolved b/Package.resolved index 9db2b46f35..8393d1e314 100644 --- a/Package.resolved +++ b/Package.resolved @@ -22,8 +22,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/jpsim/Yams", "state" : { - "revision" : "3d6871d5b4a5cd519adf233fbb576e0a2af71c17", - "version" : "5.4.0" + "revision" : "9281f8c99aff4f4a55dce22ae29b1181c935caa5", + "version" : "6.0.0" } } ], From ffbabc9dceda1e72b5cc160e15900c3c80a95d79 Mon Sep 17 00:00:00 2001 From: Doug Date: Tue, 20 May 2025 16:23:05 +0100 Subject: [PATCH 5/5] More updated snapshots. --- .../serverConfirmationScreen.Login-iPad-en-GB.png | 4 ++-- .../serverConfirmationScreen.Login-iPad-pseudo.png | 4 ++-- .../serverConfirmationScreen.Login-iPhone-16-en-GB.png | 4 ++-- .../serverConfirmationScreen.Login-iPhone-16-pseudo.png | 4 ++-- .../serverConfirmationScreen.Register-iPad-en-GB.png | 4 ++-- .../serverConfirmationScreen.Register-iPad-pseudo.png | 4 ++-- .../serverConfirmationScreen.Register-iPhone-16-en-GB.png | 4 ++-- .../serverConfirmationScreen.Register-iPhone-16-pseudo.png | 4 ++-- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPad-en-GB.png index 14a90fe890..5e4da6190b 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPad-en-GB.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPad-en-GB.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:dda96a2b756c7c250b110dc89193dc510880503a766ccbb7a00cc42f36b4959e -size 102561 +oid sha256:03bb675716240e63c16c9be349b932bc7d9a7b72253b54c597db6a6b168ff9b5 +size 102494 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPad-pseudo.png index 6fb651fc5d..6a3d796bff 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPad-pseudo.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPad-pseudo.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:bd4ab23f0c253087af15eb68953cbf7509f2ce65f5abf3eefd6568a18234e551 -size 119931 +oid sha256:9f9340c56df9fa2aa003a6bd2ced99e15ea5478ca8e18160f039c232405d44b1 +size 119843 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPhone-16-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPhone-16-en-GB.png index 9facbc8195..2de37e87c7 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPhone-16-en-GB.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPhone-16-en-GB.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:2ed8e64bec27b38226b9686e62352c41804e797b170740effc37dbc5d065903b -size 62986 +oid sha256:92090b635ca055762d5ddc18544df805de964912e04b22d5f1e094fbe9bba5ca +size 62825 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPhone-16-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPhone-16-pseudo.png index c03efed82f..c450577ad2 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPhone-16-pseudo.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Login-iPhone-16-pseudo.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:d22f168e1d09cc25655df8b573edde7782c3f66c668d5b4d3ba7ee743c5cbb4a -size 86853 +oid sha256:e44e9c31d4675aa59f582a60554daef20e243327769ce7e7b02db88da23d38cf +size 86669 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPad-en-GB.png index 317528e0fd..262bd26cac 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPad-en-GB.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPad-en-GB.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:3e82342e60533c240a9bccfdcec4ba0fad0fad2c458191d4a0bdf55eeb956265 -size 108381 +oid sha256:0b896bdca11d30a18c758c25515dcc425c5f5a94de24a0efff35cf656404824b +size 108388 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPad-pseudo.png index fb32c8e31a..c7925814ee 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPad-pseudo.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPad-pseudo.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:a52862914c8f4e530cc270e488b8e59e5e9e20ce1fc794b94c5c32da560fa282 -size 133280 +oid sha256:527eace52fe3f043d9019079a084773ac3386970cf50d25e5c5313bad3c02bbf +size 133247 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPhone-16-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPhone-16-en-GB.png index 0d46ece573..5adaf28e8a 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPhone-16-en-GB.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPhone-16-en-GB.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:267ebd6d4603e9fc6e247bc710bda29c20a7c4b14826dd6eee909efde4f26de9 -size 68125 +oid sha256:993fab8694f35677df04f4b31a2f5ff2489832450e248dfd10ec64dac858b412 +size 67983 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPhone-16-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPhone-16-pseudo.png index e22a809329..34964b9994 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPhone-16-pseudo.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/serverConfirmationScreen.Register-iPhone-16-pseudo.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:10a15b4ce667f2b90974ab62a8b2dac53cbffd6e6158d2b564bf42678169eb0e -size 97699 +oid sha256:40abda720a4f8969670c59d6ede272c557eb3f6f0b7b1a40c0cf2f9491794aeb +size 97536