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,