From 59e72fc3ee3167b99cb02f7bed5726fda4c75fa1 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 4 Sep 2026 17:28:34 +0200 Subject: [PATCH] Stop deleting the session data of a logged in account on the next login sessionPaths keeps pointing at the directories of the account which has just been logged in, and it was never reset. Adding another account calls setHomeserver(), whose rotateSessionPath() starts by deleting the directories of the previous sessionPaths, so the file and cache directories of the first account were wiped. finalizeClientCreation() now forgets the session paths once the login succeeded. It is done there rather than in clear() on purpose: after a failed attempt the directories are orphaned and must still be deleted by the rotateSessionPath() of the next attempt. Co-Authored-By: Claude Opus 5 (1M context) --- .../auth/RustMatrixAuthenticationService.kt | 4 + .../impl/RustMatrixClientFactoryTest.kt | 4 +- .../RustMatrixAuthenticationServiceTest.kt | 86 ++++++++++++++++++- 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt index 771bc60a05..681aa6ba64 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt @@ -386,6 +386,10 @@ class RustMatrixAuthenticationService( newMatrixClientObservers.forEach { it.invoke(matrixClient) } sessionStore.addSession(sessionData) + // The session paths now hold the data of the account which has just been logged in, so forget + // them: they must not be deleted by the rotateSessionPath() of the next login attempt. + sessionPaths = null + return SessionId(sessionData.userId) } diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactoryTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactoryTest.kt index 6b5b5a154a..b559b7d4d0 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactoryTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactoryTest.kt @@ -15,6 +15,7 @@ import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.impl.auth.FakeProxyProvider import io.element.android.libraries.matrix.impl.room.FakeTimelineEventFilterFactory import io.element.android.libraries.matrix.impl.storage.FakeSqliteStoreBuilderProvider +import io.element.android.libraries.matrix.impl.storage.SqliteStoreBuilderProvider import io.element.android.libraries.network.useragent.SimpleUserAgentProvider import io.element.android.libraries.sessionstorage.api.SessionStore import io.element.android.libraries.sessionstorage.test.InMemorySessionStore @@ -52,6 +53,7 @@ fun TestScope.createRustMatrixClientFactory( ), clientBuilderProvider: ClientBuilderProvider = FakeClientBuilderProvider(), workManagerScheduler: FakeWorkManagerScheduler = FakeWorkManagerScheduler(), + sqliteStoreBuilderProvider: SqliteStoreBuilderProvider = FakeSqliteStoreBuilderProvider(), ) = RustMatrixClientFactory( cacheDirectory = cacheDirectory, appCoroutineScope = backgroundScope, @@ -64,7 +66,7 @@ fun TestScope.createRustMatrixClientFactory( featureFlagService = FakeFeatureFlagService(), timelineEventFilterFactory = FakeTimelineEventFilterFactory(), clientBuilderProvider = clientBuilderProvider, - sqliteStoreBuilderProvider = FakeSqliteStoreBuilderProvider(), + sqliteStoreBuilderProvider = sqliteStoreBuilderProvider, workManagerScheduler = workManagerScheduler, clientBuilderEnterpriseHook = FakeClientBuilderEnterpriseHook(), ) diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationServiceTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationServiceTest.kt index fb4fb80e1e..d81c6021da 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationServiceTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationServiceTest.kt @@ -12,6 +12,7 @@ import com.google.common.truth.Truth.assertThat import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.enterprise.test.FakeEnterpriseService import io.element.android.libraries.featureflag.test.FakeFeatureFlagService +import io.element.android.libraries.matrix.api.paths.SessionPaths import io.element.android.libraries.matrix.impl.ClientBuilderProvider import io.element.android.libraries.matrix.impl.FakeClientBuilderProvider import io.element.android.libraries.matrix.impl.auth.qrlogin.SdkQrCodeLoginData @@ -22,6 +23,10 @@ import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeFfiHomeserver import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeFfiLoginWithQrCodeHandler import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeFfiQrCodeData import io.element.android.libraries.matrix.impl.paths.SessionPathsFactory +import io.element.android.libraries.matrix.impl.storage.FakeSqliteStoreBuilder +import io.element.android.libraries.matrix.impl.storage.FakeSqliteStoreBuilderProvider +import io.element.android.libraries.matrix.impl.storage.SqliteStoreBuilder +import io.element.android.libraries.matrix.impl.storage.SqliteStoreBuilderProvider import io.element.android.libraries.matrix.test.A_HOMESERVER_URL import io.element.android.libraries.matrix.test.A_USER_ID import io.element.android.libraries.matrix.test.auth.FakeOAuthRedirectUrlProvider @@ -33,13 +38,18 @@ import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest +import org.junit.Rule import org.junit.Test +import org.junit.rules.TemporaryFolder import org.matrix.rustcomponents.sdk.Client import org.matrix.rustcomponents.sdk.ClientBuilder import org.matrix.rustcomponents.sdk.HumanQrLoginException import java.io.File class RustMatrixAuthenticationServiceTest { + @get:Rule + val temporaryFolder = TemporaryFolder() + @Test fun `setHomeserver is successful`() = runTest { val sut = createRustMatrixAuthenticationService( @@ -177,21 +187,89 @@ class RustMatrixAuthenticationServiceTest { override fun provide(): ClientBuilder = FakeFfiClientBuilder(buildResult = clients[index++]) } + @Test + fun `a new login does not delete the session data of the account which has just been logged in`() = runTest { + val storeBuilderProvider = SessionDirectoryCreatingSqliteStoreBuilderProvider() + val sut = createRustMatrixAuthenticationService( + clientBuilderProvider = FakeSequentialClientBuilderProvider( + { aLoginFakeFfiClient() }, + { FakeFfiClient(withUtdHook = {}) }, + { aLoginFakeFfiClient() }, + ), + sessionPathsFactory = SessionPathsFactory(temporaryFolder.newFolder("base"), temporaryFolder.newFolder("cache")), + sqliteStoreBuilderProvider = storeBuilderProvider, + ) + + assertThat(sut.setHomeserver("matrix.org").isSuccess).isTrue() + assertThat(sut.login("alice", "password").getOrNull()).isEqualTo(A_USER_ID) + val loggedInSessionPaths = storeBuilderProvider.providedSessionPaths.first() + + // Adding another account rotates the session paths, which must not touch the previous account. + assertThat(sut.setHomeserver("matrix.org").isSuccess).isTrue() + + assertThat(loggedInSessionPaths.fileDirectory.exists()).isTrue() + assertThat(loggedInSessionPaths.cacheDirectory.exists()).isTrue() + } + + @Test + fun `a new login deletes the session data of a previous failed login attempt`() = runTest { + val storeBuilderProvider = SessionDirectoryCreatingSqliteStoreBuilderProvider() + val sut = createRustMatrixAuthenticationService( + clientBuilderProvider = FakeSequentialClientBuilderProvider( + { aLoginFakeFfiClient() }, + { throw IllegalStateException("Failed to build the client of the session") }, + { aLoginFakeFfiClient() }, + ), + sessionPathsFactory = SessionPathsFactory(temporaryFolder.newFolder("base"), temporaryFolder.newFolder("cache")), + sqliteStoreBuilderProvider = storeBuilderProvider, + ) + + assertThat(sut.setHomeserver("matrix.org").isSuccess).isTrue() + assertThat(sut.login("alice", "password").isFailure).isTrue() + val abandonedSessionPaths = storeBuilderProvider.providedSessionPaths.first() + + assertThat(sut.setHomeserver("matrix.org").isSuccess).isTrue() + + assertThat(abandonedSessionPaths.fileDirectory.exists()).isFalse() + assertThat(abandonedSessionPaths.cacheDirectory.exists()).isFalse() + } + + private fun aLoginFakeFfiClient() = FakeFfiClient( + homeserverLoginDetailsResult = { FakeFfiHomeserverLoginDetails() }, + loginResult = { _, _ -> }, + ) + + /** + * A [SqliteStoreBuilderProvider] which creates the session directories, like the SDK does when it + * opens its stores, and records them so that a test can assert on their lifecycle. + */ + private class SessionDirectoryCreatingSqliteStoreBuilderProvider : SqliteStoreBuilderProvider { + val providedSessionPaths = mutableListOf() + + override fun provide(sessionPaths: SessionPaths): SqliteStoreBuilder { + sessionPaths.fileDirectory.mkdirs() + sessionPaths.cacheDirectory.mkdirs() + providedSessionPaths.add(sessionPaths) + return FakeSqliteStoreBuilder() + } + } + private fun TestScope.createRustMatrixAuthenticationService( sessionStore: SessionStore = InMemorySessionStore(updateUserProfileResult = { _, _, _ -> }), clientBuilderProvider: ClientBuilderProvider = FakeClientBuilderProvider(), enterpriseService: EnterpriseService = FakeEnterpriseService(), + sessionPathsFactory: SessionPathsFactory = SessionPathsFactory(File("/base"), File("/cache")), + sqliteStoreBuilderProvider: SqliteStoreBuilderProvider = FakeSqliteStoreBuilderProvider(), ): RustMatrixAuthenticationService { - val baseDirectory = File("/base") - val cacheDirectory = File("/cache") val rustMatrixClientFactory = createRustMatrixClientFactory( - cacheDirectory = cacheDirectory, + cacheDirectory = File("/cache"), sessionStore = sessionStore, clientBuilderProvider = clientBuilderProvider, workManagerScheduler = FakeWorkManagerScheduler(submitLambda = {}), + sqliteStoreBuilderProvider = sqliteStoreBuilderProvider, ) return RustMatrixAuthenticationService( - sessionPathsFactory = SessionPathsFactory(baseDirectory, cacheDirectory), + sessionPathsFactory = sessionPathsFactory, coroutineDispatchers = testCoroutineDispatchers(), sessionStore = sessionStore, rustMatrixClientFactory = rustMatrixClientFactory,