From 6e810f2ffd104823fac5dbeec3f08d293cdbab7d Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 7 Oct 2026 14:09:05 +0200 Subject: [PATCH] Use one RustClientSessionDelegate per client, so that an auth error only affects the session it belongs to The RustMatrixClientFactory was using a single RustClientSessionDelegate for all the clients. The delegate keeps a reference to the last bound client only, and didReceiveAuthError does not tell which session is concerned. With several accounts, when the session of one account was revoked, the session of the last restored account was invalidated and its local data deleted instead, while the revoked session kept on syncing with an invalid token. Also the isLoggingOut flag was never reset, so any later auth error was ignored. The clients which are not associated to a stored session (temporary clients and clients used to perform a login) now use a NoOpClientSessionDelegate. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../matrix/impl/NoOpClientSessionDelegate.kt | 25 +++++++++++ .../matrix/impl/RustClientSessionDelegate.kt | 1 + .../matrix/impl/RustMatrixClientFactory.kt | 11 ++++- .../impl/RustTemporaryMatrixClientFactory.kt | 1 + .../auth/RustMatrixAuthenticationService.kt | 3 ++ .../impl/RustMatrixClientFactoryTest.kt | 43 +++++++++++++++++++ .../impl/fixtures/fakes/FakeFfiClient.kt | 6 ++- 7 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/NoOpClientSessionDelegate.kt diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/NoOpClientSessionDelegate.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/NoOpClientSessionDelegate.kt new file mode 100644 index 0000000000..4a2a7a4079 --- /dev/null +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/NoOpClientSessionDelegate.kt @@ -0,0 +1,25 @@ +/* + * 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. + */ + +package io.element.android.libraries.matrix.impl + +import org.matrix.rustcomponents.sdk.ClientSessionDelegate +import org.matrix.rustcomponents.sdk.Session + +/** + * A [ClientSessionDelegate] which does nothing, to be used for the clients which are not associated to a stored session, + * for instance the temporary clients, or the clients used to perform a login. The session is only stored once the login + * is done, with a client created by [RustMatrixClientFactory.create], which uses a [RustClientSessionDelegate]. + */ +internal object NoOpClientSessionDelegate : ClientSessionDelegate { + override fun saveSessionInKeychain(session: Session) = Unit + + override fun retrieveSessionFromKeychain(userId: String): Session { + // This should never be called, as it's only used for multi-process setups + error("retrieveSessionFromKeychain should never be called for Android") + } +} diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustClientSessionDelegate.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustClientSessionDelegate.kt index a5c69bf831..78fc79f0e7 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustClientSessionDelegate.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustClientSessionDelegate.kt @@ -37,6 +37,7 @@ private val loggerTag = LoggerTag("RustClientSessionDelegate") * It implements both [ClientSessionDelegate] and [ClientDelegate] to react to session data updates and auth errors. * * IMPORTANT: you must set the [client] property as soon as possible so [didReceiveAuthError] can work properly. + * Since [didReceiveAuthError] does not tell which session is concerned, each client must have its own instance. */ class RustClientSessionDelegate( private val sessionStore: SessionStore, diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactory.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactory.kt index 82e134dc24..49663bae9e 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactory.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClientFactory.kt @@ -33,6 +33,7 @@ import io.element.android.services.analytics.api.AnalyticsService import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.withContext import org.matrix.rustcomponents.sdk.ClientBuilder +import org.matrix.rustcomponents.sdk.ClientSessionDelegate import org.matrix.rustcomponents.sdk.CrossProcessLockConfig import org.matrix.rustcomponents.sdk.RequestConfig import org.matrix.rustcomponents.sdk.Session @@ -65,7 +66,11 @@ class RustMatrixClientFactory( private val clientBuilderEnterpriseHook: ClientBuilderEnterpriseHook, private val innerMatrixClientFactory: RustMatrixClient.Factory, ) { - private val sessionDelegate = RustClientSessionDelegate( + /** + * Create a new [RustClientSessionDelegate]. Each client must have its own instance, since the delegate + * keeps a reference to the client it handles the auth errors for. + */ + private fun createSessionDelegate() = RustClientSessionDelegate( sessionStore = sessionStore, appCoroutineScope = appCoroutineScope, analyticsService = analyticsService, @@ -86,11 +91,14 @@ class RustMatrixClientFactory( indexDirectory.deleteRecursively() } + // Use a dedicated delegate for this client, so that an auth error only affects this session + val sessionDelegate = createSessionDelegate() val client = getBaseClientBuilder( sessionPaths = sessionPaths, clientSecret = clientSecret, slidingSyncType = ClientBuilderSlidingSync.Restored, isMessageSearchAvailable = isMessageSearchAvailable, + sessionDelegate = sessionDelegate, ) .homeserverUrl(sessionData.homeserverUrl) .enableAutomaticBackPagination(featureFlagService.isFeatureEnabled(FeatureFlags.AutomaticBackPagination)) @@ -144,6 +152,7 @@ class RustMatrixClientFactory( clientSecret: ClientSecret?, slidingSyncType: ClientBuilderSlidingSync, isMessageSearchAvailable: Boolean, + sessionDelegate: ClientSessionDelegate, ): ClientBuilder { return clientBuilderProvider.provide() .run { diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustTemporaryMatrixClientFactory.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustTemporaryMatrixClientFactory.kt index 98de23c7c3..aaafd9abc6 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustTemporaryMatrixClientFactory.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustTemporaryMatrixClientFactory.kt @@ -35,6 +35,7 @@ class RustTemporaryMatrixClientFactory( clientSecret = null, slidingSyncType = ClientBuilderSlidingSync.Native, isMessageSearchAvailable = false, + sessionDelegate = NoOpClientSessionDelegate, ) .serverName(formattedServerName) .build() 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 1ae2a16d38..90d640e82e 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 @@ -34,6 +34,7 @@ import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.paths.SessionPaths import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import io.element.android.libraries.matrix.impl.ClientBuilderSlidingSync +import io.element.android.libraries.matrix.impl.NoOpClientSessionDelegate import io.element.android.libraries.matrix.impl.RustMatrixClientFactory import io.element.android.libraries.matrix.impl.RustTemporaryMatrixClient import io.element.android.libraries.matrix.impl.auth.qrlogin.QrErrorMapper @@ -407,6 +408,7 @@ class RustMatrixAuthenticationService( clientSecret = pendingKey, slidingSyncType = ClientBuilderSlidingSync.Discovered, isMessageSearchAvailable = isMessageSearchAvailable(), + sessionDelegate = NoOpClientSessionDelegate, ) .config() .build() @@ -440,6 +442,7 @@ class RustMatrixAuthenticationService( clientSecret = pendingKey, slidingSyncType = ClientBuilderSlidingSync.Discovered, isMessageSearchAvailable = isMessageSearchAvailable(), + sessionDelegate = NoOpClientSessionDelegate, ) .serverNameOrHomeserverUrl(baseUrlOrServerName) .build() 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 438d9f651b..1eeffae466 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,8 @@ import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.paths.SessionPaths import io.element.android.libraries.matrix.api.scanner.ContentScanner import io.element.android.libraries.matrix.impl.auth.FakeProxyProvider +import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeFfiClient +import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeFfiClientBuilder import io.element.android.libraries.matrix.impl.room.FakeTimelineEventFilterFactory import io.element.android.libraries.matrix.impl.search.RustSearchBackfillService import io.element.android.libraries.matrix.impl.search.workmanager.SearchBackfillRequestBuilder @@ -35,6 +37,7 @@ import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.junit.Test import org.matrix.rustcomponents.sdk.Client +import org.matrix.rustcomponents.sdk.ClientDelegate import java.io.File import org.matrix.rustcomponents.sdk.SyncService as ClientSyncService @@ -51,6 +54,46 @@ class RustMatrixClientFactoryTest { scheduleVacuumLambda.assertions().isCalledOnce() result.destroy() } + + @Test + fun `an auth error on a client only invalidates its own session`() = runTest { + val aliceSessionId = "@alice:server.org" + val bobSessionId = "@bob:server.org" + val sessionStore = InMemorySessionStore( + initialList = listOf( + aSessionData(sessionId = aliceSessionId, isTokenValid = true), + aSessionData(sessionId = bobSessionId, isTokenValid = true), + ), + updateUserProfileResult = { _, _, _, _ -> }, + ) + val userIds = ArrayDeque(listOf(aliceSessionId, bobSessionId)) + val delegates = mutableMapOf() + val sut = createRustMatrixClientFactory( + sessionStore = sessionStore, + workManagerScheduler = FakeWorkManagerScheduler(submitLambda = {}, cancelLambda = { _, _ -> }), + clientBuilderProvider = FakeClientBuilderProvider { + val userId = userIds.removeFirst() + FakeFfiClientBuilder( + buildResult = { + FakeFfiClient( + userId = userId, + withUtdHook = {}, + setDelegateResult = { delegates[userId] = it }, + ) + } + ) + }, + ) + val aliceClient = sut.create(sessionStore.getSession(aliceSessionId)!!) + val bobClient = sut.create(sessionStore.getSession(bobSessionId)!!) + + delegates.getValue(aliceSessionId)!!.didReceiveAuthError(isSoftLogout = false) + + assertThat(sessionStore.getSession(aliceSessionId)!!.isTokenValid).isFalse() + assertThat(sessionStore.getSession(bobSessionId)!!.isTokenValid).isTrue() + aliceClient.destroy() + bobClient.destroy() + } } fun TestScope.createRustMatrixClientFactory( diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiClient.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiClient.kt index 2a908ad885..9d6d8dd63a 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiClient.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeFfiClient.kt @@ -76,6 +76,7 @@ class FakeFfiClient( private val contentScannerResult: () -> ContentScanner = { FakeFfiContentScanner() }, private val runSearchBackfillResult: (SearchBackfillStrategy) -> TaskHandle = { lambdaError() }, private val closeResult: () -> Unit = {}, + private val setDelegateResult: (ClientDelegate?) -> Unit = {}, ) : Client(NoHandle) { override fun userId(): String = userId override fun deviceId(): String = deviceId @@ -85,7 +86,10 @@ class FakeFfiClient( override suspend fun getNotificationSettings(): NotificationSettings = notificationSettings override fun encryption(): Encryption = encryption override fun session(): Session = session - override fun setDelegate(delegate: ClientDelegate?): TaskHandle = FakeFfiTaskHandle() + override fun setDelegate(delegate: ClientDelegate?): TaskHandle { + setDelegateResult(delegate) + return FakeFfiTaskHandle() + } override suspend fun cachedAvatarUrl(): String? = null override suspend fun restoreSession(session: Session) = Unit override fun syncService(): SyncServiceBuilder = FakeFfiSyncServiceBuilder()