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

bmarty · web-flow · commit 9efa5aa4cf40 · 2025-11-07T17:28:04.000+01:00
Fix password flow when using a login link
diff --git 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()
diff --git 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)
+    }
+
+    suspend fun setUrl(url: String) {
+        setAccountProvider(createAccountProvider(url))
+    }
+
+    suspend fun setAccountProvider(data: AccountProvider) {
+        accountProvider.emit(data)
     }
 
-    fun userSelection(data: AccountProvider) {
-        accountProvider.tryEmit(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,
+        )
     }
 }
diff --git 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)
     }
 }
diff --git 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) {
diff --git 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,
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
@@ -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,
diff --git 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) {
diff --git 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,14 +29,15 @@ 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(
                 buildContext = buildContext,
                 plugins = plugins,
                 accountProviderDataSource = AccountProviderDataSource(FakeEnterpriseService()),
                 oidcActionFlow = FakeOidcActionFlow(),
+                appCoroutineScope = backgroundScope,
             )
         }
         val callback = object : LoginEntryPoint.Callback {
diff --git 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(
diff --git 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(

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ class ChangeServerPresenter(`
`65`	`65`	`throw ChangeServerError.UnsupportedServer`
`66`	`66`	`}`
`67`	`67`	`// Homeserver is valid, remember user choice`
`68`		`- accountProviderDataSource.userSelection(data)`
	`68`	`+ accountProviderDataSource.setAccountProvider(data)`
`69`	`69`	`}.runCatchingUpdatingState(changeServerAction, errorTransform = ChangeServerError::from)`
`70`	`70`	`}`
`71`	`71`	`}`