Merge pull request #5693 from element-hq/feature/bma/loginLinkPassword

Fix password flow when using a login link
2025-11-07 17:28:04 +01:00
parent 3e567d9b5a a352952a34
commit cc941d6462
10 changed files with 88 additions and 38 deletions
--- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/LoginFlowNode.kt
+++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/LoginFlowNode.kt
@@ -43,9 +43,11 @@ import io.element.android.libraries.architecture.NodeInputs
 import io.element.android.libraries.architecture.callback
 import io.element.android.libraries.architecture.createNode
 import io.element.android.libraries.architecture.inputs
+import io.element.android.libraries.di.annotations.AppCoroutineScope
 import io.element.android.libraries.matrix.api.auth.OidcDetails
 import io.element.android.libraries.oidc.api.OidcAction
 import io.element.android.libraries.oidc.api.OidcActionFlow
+import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
 import kotlinx.parcelize.Parcelize
@@ -57,6 +59,8 @@ class LoginFlowNode(
    @Assisted plugins: List<Plugin>,
    private val accountProviderDataSource: AccountProviderDataSource,
    private val oidcActionFlow: OidcActionFlow,
+    @AppCoroutineScope
+    private val appCoroutineScope: CoroutineScope,
 ) : BaseFlowNode<LoginFlowNode.NavTarget>(
    backstack = BackStack(
        initialElement = NavTarget.OnBoarding,
@@ -268,7 +272,9 @@ class LoginFlowNode(
        DisposableEffect(Unit) {
            onDispose {
                activity = null
-                accountProviderDataSource.reset()
+                appCoroutineScope.launch {
+                    accountProviderDataSource.reset()
+                }
            }
        }
        BackstackView()
--- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/accountprovider/AccountProviderDataSource.kt
+++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/accountprovider/AccountProviderDataSource.kt
@@ -21,28 +21,34 @@ import kotlinx.coroutines.flow.asStateFlow
 class AccountProviderDataSource(
    enterpriseService: EnterpriseService,
 ) {
-    private val defaultAccountProvider =
-        (enterpriseService.defaultHomeserverList().firstOrNull { it != EnterpriseService.ANY_ACCOUNT_PROVIDER } ?: AuthenticationConfig.MATRIX_ORG_URL)
-            .let { url ->
-                AccountProvider(
-                    url = url,
-                    subtitle = null,
-                    isPublic = url == AuthenticationConfig.MATRIX_ORG_URL,
-                    isMatrixOrg = url == AuthenticationConfig.MATRIX_ORG_URL,
-                )
-            }
-
-    private val accountProvider: MutableStateFlow<AccountProvider> = MutableStateFlow(
-        defaultAccountProvider
+    private val defaultAccountProvider = createAccountProvider(
+        url = enterpriseService.defaultHomeserverList()
+            .firstOrNull { it != EnterpriseService.ANY_ACCOUNT_PROVIDER }
+            ?: AuthenticationConfig.MATRIX_ORG_URL
    )

+    private val accountProvider: MutableStateFlow<AccountProvider> = MutableStateFlow(defaultAccountProvider)
+
    val flow: StateFlow<AccountProvider> = accountProvider.asStateFlow()

-    fun reset() {
-        accountProvider.tryEmit(defaultAccountProvider)
+    suspend fun reset() {
+        accountProvider.emit(defaultAccountProvider)
    }

-    fun userSelection(data: AccountProvider) {
-        accountProvider.tryEmit(data)
+    suspend fun setUrl(url: String) {
+        setAccountProvider(createAccountProvider(url))
+    }
+
+    suspend fun setAccountProvider(data: AccountProvider) {
+        accountProvider.emit(data)
+    }
+
+    private fun createAccountProvider(url: String): AccountProvider {
+        return AccountProvider(
+            url = url,
+            subtitle = null,
+            isPublic = url == AuthenticationConfig.MATRIX_ORG_URL,
+            isMatrixOrg = url == AuthenticationConfig.MATRIX_ORG_URL,
+        )
    }
 }
--- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt
+++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt
@@ -65,7 +65,7 @@ class ChangeServerPresenter(
                throw ChangeServerError.UnsupportedServer
            }
            // Homeserver is valid, remember user choice
-            accountProviderDataSource.userSelection(data)
+            accountProviderDataSource.setAccountProvider(data)
        }.runCatchingUpdatingState(changeServerAction, errorTransform = ChangeServerError::from)
    }
 }
--- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginHelper.kt
+++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginHelper.kt
@@ -25,8 +25,6 @@ import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
 import io.element.android.libraries.matrix.api.auth.OidcPrompt
 import io.element.android.libraries.oidc.api.OidcAction
 import io.element.android.libraries.oidc.api.OidcActionFlow
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.launch

 /**
 * This class is responsible for managing the login flow, including handling OIDC actions and
@@ -58,12 +56,11 @@ class LoginHelper(
        loginModeState.value = AsyncData.Uninitialized
    }

-    fun submit(
-        coroutineScope: CoroutineScope,
+    suspend fun submit(
        isAccountCreation: Boolean,
        homeserverUrl: String,
        loginHint: String?,
-    ) = coroutineScope.launch {
+    ) {
        suspend {
            authenticationService.setHomeserver(homeserverUrl).map { matrixHomeServerDetails ->
                if (matrixHomeServerDetails.supportsOidcLogin) {
--- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/chooseaccountprovider/ChooseAccountProviderPresenter.kt
+++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/chooseaccountprovider/ChooseAccountProviderPresenter.kt
@@ -22,6 +22,7 @@ import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.core.uri.ensureProtocol
 import kotlinx.collections.immutable.toImmutableList
+import kotlinx.coroutines.launch

@Inject
 class ChooseAccountProviderPresenter(
@@ -37,10 +38,9 @@ class ChooseAccountProviderPresenter(

        fun handleEvent(event: ChooseAccountProviderEvents) {
            when (event) {
-                ChooseAccountProviderEvents.Continue -> {
+                ChooseAccountProviderEvents.Continue -> localCoroutineScope.launch {
                    selectedAccountProvider?.let {
                        loginHelper.submit(
-                            coroutineScope = localCoroutineScope,
                            isAccountCreation = false,
                            homeserverUrl = it.url,
                            loginHint = null,
--- 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
@@ -17,6 +17,7 @@ import dev.zacsweers.metro.AssistedInject
 import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource
 import io.element.android.features.login.impl.login.LoginHelper
 import io.element.android.libraries.architecture.Presenter
+import kotlinx.coroutines.launch

@AssistedInject
 class ConfirmAccountProviderPresenter(
@@ -42,9 +43,8 @@ class ConfirmAccountProviderPresenter(

        fun handleEvents(event: ConfirmAccountProviderEvents) {
            when (event) {
-                ConfirmAccountProviderEvents.Continue -> {
+                ConfirmAccountProviderEvents.Continue -> localCoroutineScope.launch {
                    loginHelper.submit(
-                        coroutineScope = localCoroutineScope,
                        isAccountCreation = params.isAccountCreation,
                        homeserverUrl = accountProvider.url,
                        loginHint = null,
--- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/onboarding/OnBoardingPresenter.kt
+++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/onboarding/OnBoardingPresenter.kt
@@ -23,12 +23,14 @@ import io.element.android.appconfig.OnBoardingConfig
 import io.element.android.features.enterprise.api.EnterpriseService
 import io.element.android.features.enterprise.api.canConnectToAnyHomeserver
 import io.element.android.features.login.impl.accesscontrol.DefaultAccountProviderAccessControl
+import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource
 import io.element.android.features.login.impl.login.LoginHelper
 import io.element.android.features.rageshake.api.RageshakeFeatureAvailability
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.core.meta.BuildMeta
 import io.element.android.libraries.sessionstorage.api.SessionStore
 import io.element.android.libraries.ui.utils.MultipleTapToUnlock
+import kotlinx.coroutines.launch

@AssistedInject
 class OnBoardingPresenter(
@@ -40,6 +42,7 @@ class OnBoardingPresenter(
    private val loginHelper: LoginHelper,
    private val onBoardingLogoResIdProvider: OnBoardingLogoResIdProvider,
    private val sessionStore: SessionStore,
+    private val accountProviderDataSource: AccountProviderDataSource,
 ) : Presenter<OnBoardingState> {
    @AssistedFactory
    interface Factory {
@@ -97,12 +100,15 @@ class OnBoardingPresenter(

        fun handleEvent(event: OnBoardingEvents) {
            when (event) {
-                is OnBoardingEvents.OnSignIn -> loginHelper.submit(
-                    coroutineScope = localCoroutineScope,
-                    isAccountCreation = false,
-                    homeserverUrl = event.defaultAccountProvider,
-                    loginHint = params.loginHint?.takeIf { forcedAccountProvider == null },
-                )
+                is OnBoardingEvents.OnSignIn -> localCoroutineScope.launch {
+                    // Ensure that the current account provider is set
+                    accountProviderDataSource.setUrl(event.defaultAccountProvider)
+                    loginHelper.submit(
+                        isAccountCreation = false,
+                        homeserverUrl = event.defaultAccountProvider,
+                        loginHint = params.loginHint?.takeIf { forcedAccountProvider == null },
+                    )
+                }
                OnBoardingEvents.ClearError -> loginHelper.clearError()
                OnBoardingEvents.OnVersionClick -> {
                    if (canReportBug) {
--- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/DefaultLoginEntryPointTest.kt
+++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/DefaultLoginEntryPointTest.kt
@@ -17,6 +17,7 @@ import io.element.android.features.login.impl.accountprovider.AccountProviderDat
 import io.element.android.libraries.oidc.test.customtab.FakeOidcActionFlow
 import io.element.android.tests.testutils.lambda.lambdaError
 import io.element.android.tests.testutils.node.TestParentNode
+import kotlinx.coroutines.test.runTest
 import org.junit.Rule
 import org.junit.Test

@@ -28,7 +29,7 @@ class DefaultLoginEntryPointTest {
    val mainDispatcherRule = MainDispatcherRule()

    @Test
-    fun `test node builder`() {
+    fun `test node builder`() = runTest {
        val entryPoint = DefaultLoginEntryPoint()
        val parentNode = TestParentNode.create { buildContext, plugins ->
            LoginFlowNode(
@@ -36,6 +37,7 @@ class DefaultLoginEntryPointTest {
                plugins = plugins,
                accountProviderDataSource = AccountProviderDataSource(FakeEnterpriseService()),
                oidcActionFlow = FakeOidcActionFlow(),
+                appCoroutineScope = backgroundScope,
            )
        }
        val callback = object : LoginEntryPoint.Callback {
--- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/accountprovider/AccountProviderDataSourceTest.kt
+++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/accountprovider/AccountProviderDataSourceTest.kt
@@ -86,7 +86,30 @@ class AccountProviderDataSourceTest {
        sut.flow.test {
            val initialState = awaitItem()
            assertThat(initialState.url).isEqualTo(AuthenticationConfig.MATRIX_ORG_URL)
-            sut.userSelection(AccountProvider(url = "https://example.com"))
+            sut.setAccountProvider(AccountProvider(url = "https://example.com"))
+            val changedState = awaitItem()
+            assertThat(changedState).isEqualTo(
+                AccountProvider(
+                    url = "https://example.com",
+                    title = "example.com",
+                    subtitle = null,
+                    isPublic = false,
+                    isMatrixOrg = false,
+                )
+            )
+            sut.reset()
+            val resetState = awaitItem()
+            assertThat(resetState.url).isEqualTo(AuthenticationConfig.MATRIX_ORG_URL)
+        }
+    }
+
+    @Test
+    fun `present - set url and reset`() = runTest {
+        val sut = AccountProviderDataSource(FakeEnterpriseService())
+        sut.flow.test {
+            val initialState = awaitItem()
+            assertThat(initialState.url).isEqualTo(AuthenticationConfig.MATRIX_ORG_URL)
+            sut.setUrl(url = "https://example.com")
            val changedState = awaitItem()
            assertThat(changedState).isEqualTo(
                AccountProvider(
--- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/onboarding/OnBoardingPresenterTest.kt
+++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/onboarding/OnBoardingPresenterTest.kt
@@ -8,10 +8,12 @@
 package io.element.android.features.login.impl.screens.onboarding

 import com.google.common.truth.Truth.assertThat
+import io.element.android.appconfig.AuthenticationConfig
 import io.element.android.appconfig.OnBoardingConfig
 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
 import io.element.android.features.login.impl.login.LoginHelper
 import io.element.android.features.login.impl.web.FakeWebClientUrlForAuthenticationRetriever
 import io.element.android.features.login.impl.web.WebClientUrlForAuthenticationRetriever
@@ -24,6 +26,7 @@ import io.element.android.libraries.matrix.test.AN_ACCOUNT_PROVIDER_2
 import io.element.android.libraries.matrix.test.AN_ACCOUNT_PROVIDER_3
 import io.element.android.libraries.matrix.test.AN_EXCEPTION
 import io.element.android.libraries.matrix.test.A_HOMESERVER_URL
+import io.element.android.libraries.matrix.test.A_HOMESERVER_URL_2
 import io.element.android.libraries.matrix.test.A_LOGIN_HINT
 import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService
 import io.element.android.libraries.matrix.test.core.aBuildMeta
@@ -36,6 +39,7 @@ import io.element.android.libraries.wellknown.api.WellknownRetriever
 import io.element.android.tests.testutils.WarmUpRule
 import io.element.android.tests.testutils.test
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.test.runTest
 import org.junit.Rule
@@ -219,6 +223,7 @@ class OnBoardingPresenterTest {
                Result.failure(AN_EXCEPTION)
            },
        )
+        val accountProviderDataSource = AccountProviderDataSource(FakeEnterpriseService())
        val presenter = createPresenter(
            params = OnBoardingNode.Params(
                accountProvider = A_HOMESERVER_URL,
@@ -230,14 +235,17 @@ class OnBoardingPresenterTest {
            loginHelper = createLoginHelper(
                authenticationService = authenticationService,
            ),
+            accountProviderDataSource = accountProviderDataSource,
        )
        presenter.test {
            skipItems(3)
            awaitItem().also {
                assertThat(it.defaultAccountProvider).isEqualTo(A_HOMESERVER_URL)
-                it.eventSink(OnBoardingEvents.OnSignIn(A_HOMESERVER_URL))
+                assertThat(accountProviderDataSource.flow.first().url).isEqualTo(AuthenticationConfig.MATRIX_ORG_URL)
+                it.eventSink(OnBoardingEvents.OnSignIn(A_HOMESERVER_URL_2))
                skipItems(1) // Loading
-
+                // Account data source has been updated
+                assertThat(accountProviderDataSource.flow.first().url).isEqualTo(A_HOMESERVER_URL_2)
                // Check an error was returned
                val submittedState = awaitItem()
                assertThat(submittedState.loginMode).isInstanceOf(AsyncData.Failure::class.java)
@@ -260,6 +268,7 @@ private fun createPresenter(
    loginHelper: LoginHelper = createLoginHelper(),
    onBoardingLogoResIdProvider: OnBoardingLogoResIdProvider = OnBoardingLogoResIdProvider { null },
    sessionStore: SessionStore = InMemorySessionStore(),
+    accountProviderDataSource: AccountProviderDataSource = AccountProviderDataSource(FakeEnterpriseService()),
 ) = OnBoardingPresenter(
    params = params,
    buildMeta = buildMeta,
@@ -272,6 +281,7 @@ private fun createPresenter(
    loginHelper = loginHelper,
    onBoardingLogoResIdProvider = onBoardingLogoResIdProvider,
    sessionStore = sessionStore,
+    accountProviderDataSource = accountProviderDataSource,
 )

 fun createLoginHelper(