From 1b2d5b9c92e2ab6efbbfbaff8989f9ccce2c4303 Mon Sep 17 00:00:00 2001 From: Jenna Vassar Date: Tue, 18 Aug 2026 17:03:39 -0700 Subject: [PATCH] Complete account providers without the https scheme and include the enterprise allow-list --- .../ConfirmAccountProviderPresenter.kt | 44 ++++++++---- .../ConfirmAccountProviderView.kt | 22 ++++-- .../ConfirmAccountProviderPresenterTest.kt | 67 ++++++++++++++++--- 3 files changed, 105 insertions(+), 28 deletions(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenter.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenter.kt index e025780bc2..21bc9077dd 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenter.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenter.kt @@ -21,6 +21,7 @@ import dev.zacsweers.metro.Assisted import dev.zacsweers.metro.AssistedFactory import dev.zacsweers.metro.AssistedInject import io.element.android.appconfig.AuthenticationConfig +import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.login.impl.accountprovider.AccountProvider import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource import io.element.android.features.login.impl.changeserver.ChangeServerEvents @@ -38,6 +39,7 @@ class ConfirmAccountProviderPresenter( private val loginModePresenter: Presenter, private val changeServerPresenter: Presenter, private val appPreferencesStore: AppPreferencesStore, + private val enterpriseService: EnterpriseService, ) : Presenter { data class Params( val isAccountCreation: Boolean, @@ -55,22 +57,28 @@ class ConfirmAccountProviderPresenter( val changeServerState = changeServerPresenter.present() val homeserverHistory by appPreferencesStore.getHomeserverHistoryFlow().collectAsState(emptyList()) - // Editable input, seeded from the current (history-defaulted) account provider until the user edits it. - var userInput by rememberSaveable { mutableStateOf(null) } - val accountProviderInput = userInput ?: accountProvider.url + // The account providers offered for autocomplete: previously-used ones first, then the + // enterprise/MDM-configured allow-list, then matrix.org (always available, even before any sign-in). + // The "*" wildcard is a routing marker, not a real provider, so it is filtered out. Everything is + // rendered without the https:// scheme (added back at connection time). + val autocompleteCandidates = remember(homeserverHistory) { + (homeserverHistory + enterpriseService.homeserverAllowList() + AuthenticationConfig.MATRIX_ORG_URL) + .filter { it != EnterpriseService.ANY_ACCOUNT_PROVIDER } + .map { it.withoutScheme() } + .distinct() + } - // Offer an account provider that the current input is a prefix of: the most recent previously-used - // one first, then matrix.org. matrix.org is always available for autocomplete even before any - // sign-in, offered both with and without the https:// scheme so it completes whether the user starts - // typing "matrix.org" or "https://matrix.org". - val accountProviderSuggestion = remember(accountProviderInput, homeserverHistory) { + // Editable input, seeded from the current (history-defaulted) account provider until the user edits it. + // Displayed without the scheme, so the field shows e.g. "matrix.org" rather than "https://matrix.org". + var userInput by rememberSaveable { mutableStateOf(null) } + val accountProviderInput = userInput ?: accountProvider.url.withoutScheme() + + // Offer the first candidate that the current input is a (case-insensitive) prefix of. + val accountProviderSuggestion = remember(accountProviderInput, autocompleteCandidates) { val input = accountProviderInput.trim() - val candidates = homeserverHistory + - AuthenticationConfig.MATRIX_ORG_URL + - AuthenticationConfig.MATRIX_ORG_URL.removePrefix("https://") input.takeIf { it.isNotEmpty() } ?.let { prefix -> - candidates.firstOrNull { it.length > prefix.length && it.startsWith(prefix, ignoreCase = true) } + autocompleteCandidates.firstOrNull { it.length > prefix.length && it.startsWith(prefix, ignoreCase = true) } } } @@ -104,8 +112,12 @@ class ConfirmAccountProviderPresenter( // Validate (and persist) the chosen account provider before proceeding. This also enforces the // account-provider access control, which the login submit does not run on its own. is ConfirmAccountProviderEvents.Continue -> { + // Apply the accepted account provider (any accepted completion) back into the field so it + // renders the full server rather than the typed prefix, and survives the OAuth round-trip. + val accountProvider = event.accountProvider.trim() + userInput = accountProvider // Default the scheme to https:// so entering a bare host (e.g. "matrix.org") works. - val accountProviderUrl = event.accountProvider.trim().ensureProtocol() + val accountProviderUrl = accountProvider.ensureProtocol() submittedAccountProvider = accountProviderUrl changeServerState.eventSink( ChangeServerEvents.ChangeServer(AccountProvider(url = accountProviderUrl)) @@ -128,3 +140,9 @@ class ConfirmAccountProviderPresenter( ) } } + +/** + * The host part of an account provider URL, i.e. the value with any `https://` / `http://` scheme removed. + * Returns the input unchanged when it has no scheme (e.g. `matrix.org`). + */ +private fun String.withoutScheme(): String = substringAfter("://") diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderView.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderView.kt index 9a2e2819cc..c81fd529ef 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderView.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderView.kt @@ -50,7 +50,6 @@ import io.element.android.libraries.designsystem.atomic.molecules.IconTitleSubti import io.element.android.libraries.designsystem.atomic.pages.HeaderFooterPage import io.element.android.libraries.designsystem.components.BigIcon import io.element.android.libraries.designsystem.components.form.textFieldState -import io.element.android.libraries.designsystem.modifiers.onTabOrEnterKeyFocusNext import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.PreviewsDayNight import io.element.android.libraries.designsystem.theme.components.Button @@ -82,8 +81,9 @@ fun ConfirmAccountProviderView( val keyboardController = LocalSoftwareKeyboardController.current var input by textFieldState(stateValue = state.accountProviderInput) + // Inline autocomplete: the un-typed remainder of the suggested account provider (rendered in grey and - // accepted on Continue). Only valid while the suggestion still starts with the current input. + // accepted on Continue / Enter). Only valid while the suggestion still starts with the current input. val suggestionSuffix = state.accountProviderSuggestion ?.takeIf { it.length > input.length && it.startsWith(input, ignoreCase = true) } ?.substring(input.length) @@ -156,7 +156,6 @@ fun ConfirmAccountProviderView( .fillMaxWidth() .padding(top = 40.dp) .focusRequester(focusRequester) - .onTabOrEnterKeyFocusNext(focusManager) .testTag(TestTags.changeServerServer), label = stringResource(id = CommonStrings.screen_change_server_textfield_header), placeholder = stringResource(id = CommonStrings.screen_change_server_textfield_placeholder), @@ -172,7 +171,22 @@ fun ConfirmAccountProviderView( keyboardType = KeyboardType.Uri, imeAction = ImeAction.Done, ), - keyboardActions = KeyboardActions(onDone = { submit() }), + keyboardActions = KeyboardActions( + onDone = { + if (suggestionSuffix.isNotEmpty()) { + // A completion is showing: accept it into the field and drop focus, dismissing the + // keyboard. This shows the completed server (with no stray mid-text cursor, since the + // field is no longer focused) ready to sign in with Continue. When no completion is + // showing, Done submits directly. + val accepted = input + suggestionSuffix + input = accepted + eventSink(ConfirmAccountProviderEvents.UserInputChanged(accepted)) + focusManager.clearFocus(force = true) + } else { + submit() + } + } + ), singleLine = true, trailingIcon = if (input.isNotEmpty()) { { diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt index 826c5cbfdf..229fc375a2 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt @@ -11,6 +11,7 @@ package io.element.android.features.login.impl.screens.confirmaccountprovider import app.cash.turbine.ReceiveTurbine import com.google.common.truth.Truth.assertThat import io.element.android.appconfig.AuthenticationConfig +import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.enterprise.test.FakeEnterpriseService import io.element.android.features.login.impl.accesscontrol.DefaultAccountProviderAccessControl import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource @@ -50,7 +51,7 @@ class ConfirmAccountProviderPresenterTest { val initialState = awaitItem() assertThat(initialState.isAccountCreation).isFalse() assertThat(initialState.submitEnabled).isTrue() - assertThat(initialState.accountProviderInput).isEqualTo(AuthenticationConfig.MATRIX_ORG_URL) + assertThat(initialState.accountProviderInput).isEqualTo("matrix.org") assertThat(initialState.loginModeState.loginMode).isEqualTo(AsyncData.Uninitialized) } } @@ -315,9 +316,10 @@ class ConfirmAccountProviderPresenterTest { presenter.test { val initialState = awaitItem() assertThat(initialState.accountProviderSuggestion).isNull() - initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("https://random")) - val suggestionState = awaitState { it.accountProviderInput == "https://random" } - assertThat(suggestionState.accountProviderSuggestion).isEqualTo("https://randomcommunity.org") + initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("random")) + val suggestionState = awaitState { it.accountProviderInput == "random" } + // Completion is offered without the https:// scheme, even though history stores the full URL. + assertThat(suggestionState.accountProviderSuggestion).isEqualTo("randomcommunity.org") } } @@ -338,9 +340,9 @@ class ConfirmAccountProviderPresenterTest { ) presenter.test { val initialState = awaitItem() - initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("https://random")) + initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("random")) val suggestionState = awaitState { it.accountProviderSuggestion != null } - suggestionState.eventSink(ConfirmAccountProviderEvents.Continue("https://randomcommunity.org")) + suggestionState.eventSink(ConfirmAccountProviderEvents.Continue("randomcommunity.org")) awaitLoginMode { it is AsyncData.Success } assertThat(submittedUrls.first()).isEqualTo("https://randomcommunity.org") cancelAndIgnoreRemainingEvents() @@ -348,15 +350,40 @@ class ConfirmAccountProviderPresenterTest { } @Test - fun `present - offers matrix_org as an autocomplete suggestion even without any history`() = runTest { + fun `present - continue applies the completed account provider back into the field`() = runTest { + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { Result.success(aMatrixHomeServerDetails(supportsPasswordLogin = true)) }, + ) val presenter = createConfirmAccountProviderPresenter( - appPreferencesStore = InMemoryAppPreferencesStore(homeserverHistory = emptyList()), + matrixAuthenticationService = authenticationService, + appPreferencesStore = InMemoryAppPreferencesStore(homeserverHistory = listOf("https://randomcommunity.org")), ) presenter.test { val initialState = awaitItem() - initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("https://matr")) - val suggestionState = awaitState { it.accountProviderInput == "https://matr" } - assertThat(suggestionState.accountProviderSuggestion).isEqualTo(AuthenticationConfig.MATRIX_ORG_URL) + initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("random")) + val suggestionState = awaitState { it.accountProviderSuggestion != null } + suggestionState.eventSink(ConfirmAccountProviderEvents.Continue("randomcommunity.org")) + // The field now renders the full accepted server rather than the typed prefix. + val appliedState = awaitState { it.accountProviderInput == "randomcommunity.org" } + assertThat(appliedState.accountProviderInput).isEqualTo("randomcommunity.org") + cancelAndIgnoreRemainingEvents() + } + } + + @Test + fun `present - offers the enterprise allow-list servers as autocomplete suggestions`() = runTest { + val presenter = createConfirmAccountProviderPresenter( + appPreferencesStore = InMemoryAppPreferencesStore(homeserverHistory = emptyList()), + enterpriseService = FakeEnterpriseService( + // Preferred servers plus the "*" (any) wildcard: the wildcard is filtered out, the rest complete. + defaultHomeserverListResult = { listOf("https://element.io", EnterpriseService.ANY_ACCOUNT_PROVIDER) }, + ), + ) + presenter.test { + val initialState = awaitItem() + initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("ele")) + val suggestionState = awaitState { it.accountProviderInput == "ele" } + assertThat(suggestionState.accountProviderSuggestion).isEqualTo("element.io") } } @@ -373,6 +400,22 @@ class ConfirmAccountProviderPresenterTest { } } + @Test + fun `present - does not autocomplete a scheme-prefixed input`() = runTest { + // Candidates are bare hosts, so a scheme-prefixed input never completes them - the user completes by + // typing the bare host. This also avoids ever suggesting an insecure http scheme. + val presenter = createConfirmAccountProviderPresenter( + appPreferencesStore = InMemoryAppPreferencesStore(homeserverHistory = emptyList()), + ) + presenter.test { + val initialState = awaitItem() + initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("https://mat")) + assertThat(awaitState { it.accountProviderInput == "https://mat" }.accountProviderSuggestion).isNull() + initialState.eventSink(ConfirmAccountProviderEvents.UserInputChanged("http://mat")) + assertThat(awaitState { it.accountProviderInput == "http://mat" }.accountProviderSuggestion).isNull() + } + } + @Test fun `present - continue prepends https to a bare account provider`() = runTest { val submittedUrls = mutableListOf() @@ -440,10 +483,12 @@ class ConfirmAccountProviderPresenterTest { matrixAuthenticationService: MatrixAuthenticationService = FakeMatrixAuthenticationService(), defaultOAuthActionFlow: OAuthActionFlow = FakeOAuthActionFlow(), appPreferencesStore: AppPreferencesStore = InMemoryAppPreferencesStore(), + enterpriseService: EnterpriseService = FakeEnterpriseService(), ) = ConfirmAccountProviderPresenter( params = params, accountProviderDataSource = accountProviderDataSource, appPreferencesStore = appPreferencesStore, + enterpriseService = enterpriseService, loginModePresenter = createLoginModePresenter( authenticationService = matrixAuthenticationService, oAuthActionFlow = defaultOAuthActionFlow,