From d471241da871d72e9c5c904d383161777ebe4dfb Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 20 Aug 2026 13:57:55 +0200 Subject: [PATCH] Fix account provisioning deep link being ignored when it cold starts the app At cold start two writers set the root nav target of RootFlowNode: the intent path (handleIntent -> onLoginLink -> NotLoggedInFlow(params)) and the first nav state emission (NotLoggedIn -> NotLoggedInFlow(null)). safeRoot() only dedupes on strict equality, so the two targets do not cancel each other out and the last writer wins, silently dropping the login params. Keep the params coming from the intent as pending state on the node, and let the nav state observer take them into account, so that the root is derived from both the logged in state and the pending login params, and both orderings converge on the same target. The pending params are cleared when leaving the not logged in flow, so that a later sign out does not resurrect a stale provisioning link. Also log when switching to the not logged in flow, since the drop was silent. The login hint can contain an email address, so only its presence is logged. Closes #7482 Co-Authored-By: Claude Opus 5 (1M context) --- appnav/build.gradle.kts | 3 + .../io/element/android/appnav/RootFlowNode.kt | 14 +- .../android/appnav/RootFlowNodeTest.kt | 267 ++++++++++++++++++ 3 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 appnav/src/test/kotlin/io/element/android/appnav/RootFlowNodeTest.kt diff --git a/appnav/build.gradle.kts b/appnav/build.gradle.kts index 054e9525c7..64aa2a6fc5 100644 --- a/appnav/build.gradle.kts +++ b/appnav/build.gradle.kts @@ -63,11 +63,14 @@ dependencies { testImplementation(projects.features.share.test) testImplementation(projects.libraries.featureflag.test) testImplementation(projects.libraries.matrix.test) + testImplementation(projects.libraries.matrixmedia.test) testImplementation(projects.libraries.oauth.test) testImplementation(projects.libraries.permissions.test) testImplementation(projects.libraries.preferences.test) testImplementation(projects.libraries.push.test) testImplementation(projects.libraries.pushproviders.test) + testImplementation(projects.libraries.sessionStorage.test) + testImplementation(projects.features.announcement.test) testImplementation(projects.features.forward.test) testImplementation(projects.features.messages.test) testImplementation(projects.features.networkmonitor.test) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt index acf7b66db9..19ec427303 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt @@ -109,6 +109,13 @@ class RootFlowNode( buildContext = buildContext, plugins = plugins ) { + /** + * Login params coming from a launch or new [Intent], waiting to be consumed by the not logged in flow. + * Kept here so that the root nav target can be computed from both the logged in state and the pending + * login params, whatever the order in which the intent and the first nav state emission are processed. + */ + private var pendingLoginParams: LoginParams? = null + override fun onBuilt() { analyticsColdStartWatcher.start() appCoroutineScope.launch { @@ -152,7 +159,7 @@ class RootFlowNode( } } LoggedInState.NotLoggedIn -> { - switchToNotLoggedInFlow(null) + switchToNotLoggedInFlow(pendingLoginParams) } } } @@ -194,15 +201,18 @@ class RootFlowNode( } private fun switchToLoggedInFlow(sessionId: SessionId, navId: Int) { + pendingLoginParams = null backstack.safeRoot(NavTarget.LoggedInFlow(sessionId, navId)) } private fun switchToNotLoggedInFlow(params: LoginParams?) { + Timber.d("switchToNotLoggedInFlow, hasLoginParams=${params != null}") matrixSessionCache.removeAll() backstack.safeRoot(NavTarget.NotLoggedInFlow(params)) } private fun switchToSignedOutFlow(sessionId: SessionId) { + pendingLoginParams = null backstack.safeRoot(NavTarget.SignedOutFlow(sessionId)) } @@ -319,6 +329,7 @@ class RootFlowNode( } override fun onDone() { + pendingLoginParams = null backstack.pop() } } @@ -418,6 +429,7 @@ class RootFlowNode( Timber.w("Login link ignored, multi account is disabled") } } else { + pendingLoginParams = params switchToNotLoggedInFlow(params) } } else { diff --git a/appnav/src/test/kotlin/io/element/android/appnav/RootFlowNodeTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/RootFlowNodeTest.kt new file mode 100644 index 0000000000..e749ffa492 --- /dev/null +++ b/appnav/src/test/kotlin/io/element/android/appnav/RootFlowNodeTest.kt @@ -0,0 +1,267 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +@file:OptIn(ExperimentalCoroutinesApi::class) + +package io.element.android.appnav + +import android.content.Intent +import android.net.Uri +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import com.bumble.appyx.core.modality.AncestryInfo +import com.bumble.appyx.core.modality.BuildContext +import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel +import com.bumble.appyx.core.node.Node +import com.bumble.appyx.core.node.ParentNode +import com.bumble.appyx.core.node.node +import com.bumble.appyx.navmodel.backstack.activeElement +import com.bumble.appyx.testing.junit4.util.MainDispatcherRule +import com.bumble.appyx.testing.unit.common.helper.parentNodeTestHelper +import com.bumble.appyx.utils.customisations.NodeCustomisationDirectoryImpl +import com.google.common.truth.Truth.assertThat +import io.element.android.appnav.di.MatrixSessionCache +import io.element.android.appnav.di.SyncOrchestrator +import io.element.android.appnav.intent.IntentResolver +import io.element.android.appnav.root.RootNavStateFlowFactory +import io.element.android.appnav.root.RootPresenter +import io.element.android.features.login.api.LoginEntryPoint +import io.element.android.features.login.api.LoginParams +import io.element.android.features.login.api.accesscontrol.AccountProviderAccessControl +import io.element.android.features.login.test.FakeLoginIntentResolver +import io.element.android.features.preferences.api.CacheService +import io.element.android.features.rageshake.api.bugreport.BugReportEntryPoint +import io.element.android.features.rageshake.test.logs.FakeAnnouncementService +import io.element.android.features.share.test.FakeShareIntentHandler +import io.element.android.features.signedout.api.SignedOutEntryPoint +import io.element.android.libraries.accountselect.api.AccountSelectEntryPoint +import io.element.android.libraries.architecture.AssistedNodeFactory +import io.element.android.libraries.architecture.NodeFactoriesBindings +import io.element.android.libraries.architecture.Presenter +import io.element.android.libraries.deeplink.api.DeeplinkParser +import io.element.android.libraries.di.DependencyInjectionGraphOwner +import io.element.android.libraries.featureflag.test.FakeFeatureFlagService +import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.SessionId +import io.element.android.libraries.matrix.test.FakeSdkMetadata +import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService +import io.element.android.libraries.matrix.test.permalink.FakePermalinkParser +import io.element.android.libraries.matrix.ui.media.test.FakeImageLoaderHolder +import io.element.android.libraries.oauth.api.OAuthAction +import io.element.android.libraries.oauth.api.OAuthActionFlow +import io.element.android.libraries.oauth.test.FakeOAuthIntentResolver +import io.element.android.libraries.preferences.test.FakeSessionPreferencesStoreFactory +import io.element.android.libraries.sessionstorage.api.SessionStore +import io.element.android.libraries.sessionstorage.test.InMemorySessionStore +import io.element.android.services.analytics.test.FakeAnalyticsService +import io.element.android.services.analytics.test.watchers.FakeAnalyticsColdStartWatcher +import io.element.android.services.apperror.test.FakeAppErrorStateService +import io.element.android.tests.testutils.robolectric.RobolectricTest +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.FlowCollector +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Rule +import org.junit.Test +import kotlin.reflect.KClass + +class RootFlowNodeTest : RobolectricTest() { + @get:Rule + val instantTaskExecutorRule = InstantTaskExecutorRule() + + @get:Rule + val mainDispatcherRule = MainDispatcherRule() + + @Test + fun `given no session, when a login link is handled before the nav state is observed, then the login params are kept`() = runTest { + val loginEntryPoint = FakeLoginEntryPoint() + val rootFlowNode = createRootFlowNode(loginEntryPoint = loginEntryPoint) + rootFlowNode.parentNodeTestHelper() + // The intent is handled first, this is what happens when the deep link cold starts the app. + rootFlowNode.handleIntent(aLoginIntent()) + assertThat(rootFlowNode.backstack.activeElement).isEqualTo(RootFlowNode.NavTarget.NotLoggedInFlow(A_LOGIN_PARAMS)) + // Then the first nav state emission lands, it must not override the login params. + runCurrent() + assertThat(rootFlowNode.backstack.activeElement).isEqualTo(RootFlowNode.NavTarget.NotLoggedInFlow(A_LOGIN_PARAMS)) + assertThat(loginEntryPoint.params).isEqualTo(A_LOGIN_ENTRY_POINT_PARAMS) + } + + @Test + fun `given no session, when a login link is handled after the nav state is observed, then the login params are applied`() = runTest { + val loginEntryPoint = FakeLoginEntryPoint() + val rootFlowNode = createRootFlowNode(loginEntryPoint = loginEntryPoint) + rootFlowNode.parentNodeTestHelper() + // The nav state is observed first, this is what happens when the app is already running. + runCurrent() + assertThat(rootFlowNode.backstack.activeElement).isEqualTo(RootFlowNode.NavTarget.NotLoggedInFlow(null)) + rootFlowNode.handleIntent(aLoginIntent()) + runCurrent() + assertThat(rootFlowNode.backstack.activeElement).isEqualTo(RootFlowNode.NavTarget.NotLoggedInFlow(A_LOGIN_PARAMS)) + assertThat(loginEntryPoint.params).isEqualTo(A_LOGIN_ENTRY_POINT_PARAMS) + } + + private fun aLoginIntent() = Intent(Intent.ACTION_VIEW, Uri.parse(A_LOGIN_LINK)) + + private fun TestScope.createRootFlowNode( + loginEntryPoint: LoginEntryPoint = FakeLoginEntryPoint(), + sessionStore: SessionStore = InMemorySessionStore(), + ): RootFlowNode { + val matrixSessionCache = MatrixSessionCache( + authenticationService = FakeMatrixAuthenticationService(), + syncOrchestratorFactory = FakeSyncOrchestratorFactory(), + analyticsService = FakeAnalyticsService(), + ) + val parentNode = FakeParentNode( + graph = FakeNodeFactoriesBindings( + mapOf( + NotLoggedInFlowNode::class to AssistedNodeFactory { buildContext, plugins -> + NotLoggedInFlowNode( + buildContext = buildContext, + plugins = plugins, + loginEntryPoint = loginEntryPoint, + imageLoaderHolder = FakeImageLoaderHolder(), + analyticsColdStartWatcher = FakeAnalyticsColdStartWatcher(), + ) + } + ) + ) + ) + return RootFlowNode( + buildContext = BuildContext( + ancestryInfo = AncestryInfo.Child(anchor = parentNode), + savedStateMap = null, + customisations = NodeCustomisationDirectoryImpl(), + ), + plugins = emptyList(), + sessionStore = sessionStore, + accountProviderAccessControl = FakeAccountProviderAccessControl(), + navStateFlowFactory = RootNavStateFlowFactory( + sessionStore = sessionStore, + cacheService = FakeCacheService(), + matrixSessionCache = matrixSessionCache, + imageLoaderHolder = FakeImageLoaderHolder(), + sessionPreferencesStoreFactory = FakeSessionPreferencesStoreFactory(), + ), + matrixSessionCache = matrixSessionCache, + presenter = RootPresenter( + crashDetectionPresenter = NotUsedPresenter(), + rageshakeDetectionPresenter = NotUsedPresenter(), + appErrorStateService = FakeAppErrorStateService(), + analyticsService = FakeAnalyticsService(), + sdkMetadata = FakeSdkMetadata("sha"), + ), + bugReportEntryPoint = FakeBugReportEntryPoint(), + signedOutEntryPoint = FakeSignedOutEntryPoint(), + accountSelectEntryPoint = FakeAccountSelectEntryPoint(), + intentResolver = IntentResolver( + deeplinkParser = DeeplinkParser { null }, + loginIntentResolver = FakeLoginIntentResolver { A_LOGIN_PARAMS }, + oAuthIntentResolver = FakeOAuthIntentResolver { null }, + permalinkParser = FakePermalinkParser(), + shareIntentHandler = FakeShareIntentHandler(), + ), + oAuthActionFlow = FakeOAuthActionFlow(), + featureFlagService = FakeFeatureFlagService(), + announcementService = FakeAnnouncementService(), + analyticsService = FakeAnalyticsService(), + analyticsColdStartWatcher = FakeAnalyticsColdStartWatcher(), + appCoroutineScope = backgroundScope, + ) + } +} + +private const val A_LOGIN_LINK = "https://mobile.element.io/element/?account_provider=example.com&login_hint=mxid:@alice:example.com" + +private val A_LOGIN_PARAMS = LoginParams( + accountProvider = "example.com", + loginHint = "mxid:@alice:example.com", +) + +private val A_LOGIN_ENTRY_POINT_PARAMS = LoginEntryPoint.Params( + accountProvider = A_LOGIN_PARAMS.accountProvider, + loginHint = A_LOGIN_PARAMS.loginHint, +) + +/** + * The presenters are only used when the [Node] is rendered, which does not happen in these tests. + */ +private class NotUsedPresenter : Presenter { + @Composable + override fun present(): T = error("Not used") +} + +private class FakeCacheService : CacheService { + override val clearedCacheEventFlow = emptyFlow() +} + +private class FakeSyncOrchestratorFactory : SyncOrchestrator.Factory { + override fun create(matrixClient: MatrixClient, sessionCoroutineScope: CoroutineScope): SyncOrchestrator = error("Not used") +} + +private class FakeOAuthActionFlow : OAuthActionFlow { + override fun post(oAuthAction: OAuthAction) = Unit + override suspend fun collect(collector: FlowCollector) = Unit + override fun reset() = Unit +} + +private class FakeAccountProviderAccessControl : AccountProviderAccessControl { + override suspend fun isAllowedToConnectToAccountProvider(accountProviderUrl: String) = true +} + +private class FakeBugReportEntryPoint : BugReportEntryPoint { + override fun createNode(parentNode: Node, buildContext: BuildContext, callback: BugReportEntryPoint.Callback): Node = node(buildContext) {} +} + +private class FakeSignedOutEntryPoint : SignedOutEntryPoint { + override fun createNode(parentNode: Node, buildContext: BuildContext, params: SignedOutEntryPoint.Params): Node = node(buildContext) {} +} + +private class FakeAccountSelectEntryPoint : AccountSelectEntryPoint { + override fun createNode(parentNode: Node, buildContext: BuildContext, callback: AccountSelectEntryPoint.Callback): Node = node(buildContext) {} +} + +private class FakeLoginEntryPoint : LoginEntryPoint { + var params: LoginEntryPoint.Params? = null + private set + + override fun createNode( + parentNode: Node, + buildContext: BuildContext, + params: LoginEntryPoint.Params, + callback: LoginEntryPoint.Callback, + ): Node { + this.params = params + return node(buildContext) {} + } +} + +private class FakeNodeFactoriesBindings( + private val nodeFactories: Map, AssistedNodeFactory<*>>, +) : NodeFactoriesBindings { + override fun nodeFactories() = nodeFactories +} + +/** + * [RootFlowNode] creates its children using the dependency injection graph of its ancestors, so it needs a parent + * owning a graph able to create a [NotLoggedInFlowNode]. + */ +private class FakeParentNode( + override val graph: Any, +) : ParentNode( + navModel = PermanentNavModel(navTargets = setOf(Unit), savedStateMap = null), + buildContext = BuildContext.root(savedStateMap = null), +), + DependencyInjectionGraphOwner { + override fun resolve(navTarget: Unit, buildContext: BuildContext): Node = node(buildContext) {} + + @Composable + override fun View(modifier: Modifier) = Unit +}