mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-11-04 01:19:33 +00:00
Merge pull request #7877 from element-hq/bugfix/bma/sessionDelegatePerClient
Fix the wrong account being signed out when the session of another account is revoked
This commit is contained in:
+25
@@ -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")
|
||||||
|
}
|
||||||
|
}
|
||||||
+1
@@ -37,6 +37,7 @@ private val loggerTag = LoggerTag("RustClientSessionDelegate")
|
|||||||
* It implements both [ClientSessionDelegate] and [ClientDelegate] to react to session data updates and auth errors.
|
* 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.
|
* 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(
|
class RustClientSessionDelegate(
|
||||||
private val sessionStore: SessionStore,
|
private val sessionStore: SessionStore,
|
||||||
|
|||||||
+10
-1
@@ -33,6 +33,7 @@ import io.element.android.services.analytics.api.AnalyticsService
|
|||||||
import kotlinx.coroutines.CoroutineScope
|
import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.withContext
|
import kotlinx.coroutines.withContext
|
||||||
import org.matrix.rustcomponents.sdk.ClientBuilder
|
import org.matrix.rustcomponents.sdk.ClientBuilder
|
||||||
|
import org.matrix.rustcomponents.sdk.ClientSessionDelegate
|
||||||
import org.matrix.rustcomponents.sdk.CrossProcessLockConfig
|
import org.matrix.rustcomponents.sdk.CrossProcessLockConfig
|
||||||
import org.matrix.rustcomponents.sdk.RequestConfig
|
import org.matrix.rustcomponents.sdk.RequestConfig
|
||||||
import org.matrix.rustcomponents.sdk.Session
|
import org.matrix.rustcomponents.sdk.Session
|
||||||
@@ -65,7 +66,11 @@ class RustMatrixClientFactory(
|
|||||||
private val clientBuilderEnterpriseHook: ClientBuilderEnterpriseHook,
|
private val clientBuilderEnterpriseHook: ClientBuilderEnterpriseHook,
|
||||||
private val innerMatrixClientFactory: RustMatrixClient.Factory,
|
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,
|
sessionStore = sessionStore,
|
||||||
appCoroutineScope = appCoroutineScope,
|
appCoroutineScope = appCoroutineScope,
|
||||||
analyticsService = analyticsService,
|
analyticsService = analyticsService,
|
||||||
@@ -86,11 +91,14 @@ class RustMatrixClientFactory(
|
|||||||
indexDirectory.deleteRecursively()
|
indexDirectory.deleteRecursively()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Use a dedicated delegate for this client, so that an auth error only affects this session
|
||||||
|
val sessionDelegate = createSessionDelegate()
|
||||||
val client = getBaseClientBuilder(
|
val client = getBaseClientBuilder(
|
||||||
sessionPaths = sessionPaths,
|
sessionPaths = sessionPaths,
|
||||||
clientSecret = clientSecret,
|
clientSecret = clientSecret,
|
||||||
slidingSyncType = ClientBuilderSlidingSync.Restored,
|
slidingSyncType = ClientBuilderSlidingSync.Restored,
|
||||||
isMessageSearchAvailable = isMessageSearchAvailable,
|
isMessageSearchAvailable = isMessageSearchAvailable,
|
||||||
|
sessionDelegate = sessionDelegate,
|
||||||
)
|
)
|
||||||
.homeserverUrl(sessionData.homeserverUrl)
|
.homeserverUrl(sessionData.homeserverUrl)
|
||||||
.enableAutomaticBackPagination(featureFlagService.isFeatureEnabled(FeatureFlags.AutomaticBackPagination))
|
.enableAutomaticBackPagination(featureFlagService.isFeatureEnabled(FeatureFlags.AutomaticBackPagination))
|
||||||
@@ -144,6 +152,7 @@ class RustMatrixClientFactory(
|
|||||||
clientSecret: ClientSecret?,
|
clientSecret: ClientSecret?,
|
||||||
slidingSyncType: ClientBuilderSlidingSync,
|
slidingSyncType: ClientBuilderSlidingSync,
|
||||||
isMessageSearchAvailable: Boolean,
|
isMessageSearchAvailable: Boolean,
|
||||||
|
sessionDelegate: ClientSessionDelegate,
|
||||||
): ClientBuilder {
|
): ClientBuilder {
|
||||||
return clientBuilderProvider.provide()
|
return clientBuilderProvider.provide()
|
||||||
.run {
|
.run {
|
||||||
|
|||||||
+1
@@ -35,6 +35,7 @@ class RustTemporaryMatrixClientFactory(
|
|||||||
clientSecret = null,
|
clientSecret = null,
|
||||||
slidingSyncType = ClientBuilderSlidingSync.Native,
|
slidingSyncType = ClientBuilderSlidingSync.Native,
|
||||||
isMessageSearchAvailable = false,
|
isMessageSearchAvailable = false,
|
||||||
|
sessionDelegate = NoOpClientSessionDelegate,
|
||||||
)
|
)
|
||||||
.serverName(formattedServerName)
|
.serverName(formattedServerName)
|
||||||
.build()
|
.build()
|
||||||
|
|||||||
+3
@@ -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.paths.SessionPaths
|
||||||
import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus
|
import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus
|
||||||
import io.element.android.libraries.matrix.impl.ClientBuilderSlidingSync
|
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.RustMatrixClientFactory
|
||||||
import io.element.android.libraries.matrix.impl.RustTemporaryMatrixClient
|
import io.element.android.libraries.matrix.impl.RustTemporaryMatrixClient
|
||||||
import io.element.android.libraries.matrix.impl.auth.qrlogin.QrErrorMapper
|
import io.element.android.libraries.matrix.impl.auth.qrlogin.QrErrorMapper
|
||||||
@@ -407,6 +408,7 @@ class RustMatrixAuthenticationService(
|
|||||||
clientSecret = pendingKey,
|
clientSecret = pendingKey,
|
||||||
slidingSyncType = ClientBuilderSlidingSync.Discovered,
|
slidingSyncType = ClientBuilderSlidingSync.Discovered,
|
||||||
isMessageSearchAvailable = isMessageSearchAvailable(),
|
isMessageSearchAvailable = isMessageSearchAvailable(),
|
||||||
|
sessionDelegate = NoOpClientSessionDelegate,
|
||||||
)
|
)
|
||||||
.config()
|
.config()
|
||||||
.build()
|
.build()
|
||||||
@@ -440,6 +442,7 @@ class RustMatrixAuthenticationService(
|
|||||||
clientSecret = pendingKey,
|
clientSecret = pendingKey,
|
||||||
slidingSyncType = ClientBuilderSlidingSync.Discovered,
|
slidingSyncType = ClientBuilderSlidingSync.Discovered,
|
||||||
isMessageSearchAvailable = isMessageSearchAvailable(),
|
isMessageSearchAvailable = isMessageSearchAvailable(),
|
||||||
|
sessionDelegate = NoOpClientSessionDelegate,
|
||||||
)
|
)
|
||||||
.serverNameOrHomeserverUrl(baseUrlOrServerName)
|
.serverNameOrHomeserverUrl(baseUrlOrServerName)
|
||||||
.build()
|
.build()
|
||||||
|
|||||||
+43
@@ -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.paths.SessionPaths
|
||||||
import io.element.android.libraries.matrix.api.scanner.ContentScanner
|
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.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.room.FakeTimelineEventFilterFactory
|
||||||
import io.element.android.libraries.matrix.impl.search.RustSearchBackfillService
|
import io.element.android.libraries.matrix.impl.search.RustSearchBackfillService
|
||||||
import io.element.android.libraries.matrix.impl.search.workmanager.SearchBackfillRequestBuilder
|
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 kotlinx.coroutines.test.runTest
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
import org.matrix.rustcomponents.sdk.Client
|
import org.matrix.rustcomponents.sdk.Client
|
||||||
|
import org.matrix.rustcomponents.sdk.ClientDelegate
|
||||||
import java.io.File
|
import java.io.File
|
||||||
import org.matrix.rustcomponents.sdk.SyncService as ClientSyncService
|
import org.matrix.rustcomponents.sdk.SyncService as ClientSyncService
|
||||||
|
|
||||||
@@ -51,6 +54,46 @@ class RustMatrixClientFactoryTest {
|
|||||||
scheduleVacuumLambda.assertions().isCalledOnce()
|
scheduleVacuumLambda.assertions().isCalledOnce()
|
||||||
result.destroy()
|
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<String, ClientDelegate?>()
|
||||||
|
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(
|
fun TestScope.createRustMatrixClientFactory(
|
||||||
|
|||||||
+5
-1
@@ -76,6 +76,7 @@ class FakeFfiClient(
|
|||||||
private val contentScannerResult: () -> ContentScanner = { FakeFfiContentScanner() },
|
private val contentScannerResult: () -> ContentScanner = { FakeFfiContentScanner() },
|
||||||
private val runSearchBackfillResult: (SearchBackfillStrategy) -> TaskHandle = { lambdaError() },
|
private val runSearchBackfillResult: (SearchBackfillStrategy) -> TaskHandle = { lambdaError() },
|
||||||
private val closeResult: () -> Unit = {},
|
private val closeResult: () -> Unit = {},
|
||||||
|
private val setDelegateResult: (ClientDelegate?) -> Unit = {},
|
||||||
) : Client(NoHandle) {
|
) : Client(NoHandle) {
|
||||||
override fun userId(): String = userId
|
override fun userId(): String = userId
|
||||||
override fun deviceId(): String = deviceId
|
override fun deviceId(): String = deviceId
|
||||||
@@ -85,7 +86,10 @@ class FakeFfiClient(
|
|||||||
override suspend fun getNotificationSettings(): NotificationSettings = notificationSettings
|
override suspend fun getNotificationSettings(): NotificationSettings = notificationSettings
|
||||||
override fun encryption(): Encryption = encryption
|
override fun encryption(): Encryption = encryption
|
||||||
override fun session(): Session = session
|
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 cachedAvatarUrl(): String? = null
|
||||||
override suspend fun restoreSession(session: Session) = Unit
|
override suspend fun restoreSession(session: Session) = Unit
|
||||||
override fun syncService(): SyncServiceBuilder = FakeFfiSyncServiceBuilder()
|
override fun syncService(): SyncServiceBuilder = FakeFfiSyncServiceBuilder()
|
||||||
|
|||||||
Reference in New Issue
Block a user