diff --git a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt index c619629152..91f5d7da19 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt @@ -142,12 +142,12 @@ class LoggedInPresenter @Inject constructor( .also { Timber.tag(pusherTag.value).w("No distributors available") } .also { // In this case, consider the push provider is chosen. - pushService.selectPushProvider(matrixClient, pushProvider) + pushService.selectPushProvider(matrixClient.sessionId, pushProvider) } .also { pusherRegistrationState.value = AsyncData.Failure(PusherRegistrationFailure.NoDistributorsAvailable()) } pushService.registerWith(matrixClient, pushProvider, distributor) } else { - val currentPushDistributor = currentPushProvider.getCurrentDistributor(matrixClient) + val currentPushDistributor = currentPushProvider.getCurrentDistributor(matrixClient.sessionId) if (currentPushDistributor == null) { Timber.tag(pusherTag.value).d("Register with the first available distributor") val distributor = currentPushProvider.getDistributors().firstOrNull() diff --git a/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt index 961856148e..ec44876634 100644 --- a/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt +++ b/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt @@ -378,7 +378,7 @@ class LoggedInPresenterTest { val lambda = lambdaRecorder> { _, _, _ -> Result.success(Unit) } - val selectPushProviderLambda = lambdaRecorder { _, _ -> } + val selectPushProviderLambda = lambdaRecorder { _, _ -> } val sessionVerificationService = FakeSessionVerificationService( initialSessionVerifiedStatus = SessionVerifiedStatus.Verified ) @@ -408,8 +408,8 @@ class LoggedInPresenterTest { selectPushProviderLambda.assertions() .isCalledOnce() .with( - // MatrixClient - any(), + // SessionId + value(A_SESSION_ID), // PushProvider value(pushProvider), ) @@ -481,7 +481,7 @@ class LoggedInPresenterTest { registerWithLambda: (MatrixClient, PushProvider, Distributor) -> Result = { _, _, _ -> Result.success(Unit) }, - selectPushProviderLambda: (MatrixClient, PushProvider) -> Unit = { _, _ -> lambdaError() }, + selectPushProviderLambda: (SessionId, PushProvider) -> Unit = { _, _ -> lambdaError() }, currentPushProvider: () -> PushProvider? = { null }, setIgnoreRegistrationErrorLambda: (SessionId, Boolean) -> Unit = { _, _ -> lambdaError() }, ): PushService { diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt index 66574f4c28..b245b9ff9a 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt @@ -93,7 +93,7 @@ class NotificationSettingsPresenter @Inject constructor( LaunchedEffect(refreshPushProvider) { val p = pushService.getCurrentPushProvider() - val name = p?.getCurrentDistributor(matrixClient)?.name + val name = p?.getCurrentDistributor(matrixClient.sessionId)?.name currentDistributorName = if (name != null) { AsyncData.Success(name) } else { diff --git a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt index a28abbd9ef..872a968642 100644 --- a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt +++ b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt @@ -40,7 +40,7 @@ interface PushService { * To be used when there is no distributor available. */ suspend fun selectPushProvider( - matrixClient: MatrixClient, + sessionId: SessionId, pushProvider: PushProvider, ) diff --git a/libraries/push/impl/build.gradle.kts b/libraries/push/impl/build.gradle.kts index 74d6ba3182..6489f26dec 100644 --- a/libraries/push/impl/build.gradle.kts +++ b/libraries/push/impl/build.gradle.kts @@ -43,6 +43,7 @@ dependencies { implementation(projects.libraries.matrix.api) implementation(projects.libraries.matrixui) implementation(projects.libraries.preferences.api) + implementation(projects.libraries.sessionStorage.api) implementation(projects.libraries.uiStrings) implementation(projects.libraries.troubleshoot.api) implementation(projects.features.call.api) @@ -64,6 +65,7 @@ dependencies { testImplementation(libs.coroutines.test) testImplementation(projects.libraries.matrix.test) testImplementation(projects.libraries.preferences.test) + testImplementation(projects.libraries.sessionStorage.test) testImplementation(projects.libraries.push.test) testImplementation(projects.libraries.pushproviders.test) testImplementation(projects.libraries.pushstore.test) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt index 789b8e1f4e..4e91940bdc 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt @@ -9,6 +9,7 @@ package io.element.android.libraries.push.impl import com.squareup.anvil.annotations.ContributesBinding import io.element.android.libraries.di.AppScope +import io.element.android.libraries.di.SingleIn import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.push.api.GetCurrentPushProvider @@ -17,17 +18,27 @@ import io.element.android.libraries.push.impl.test.TestPush import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider import io.element.android.libraries.pushstore.api.UserPushStoreFactory +import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretStore +import io.element.android.libraries.sessionstorage.api.observer.SessionListener +import io.element.android.libraries.sessionstorage.api.observer.SessionObserver import kotlinx.coroutines.flow.Flow import timber.log.Timber import javax.inject.Inject -@ContributesBinding(AppScope::class) +@ContributesBinding(AppScope::class, boundType = PushService::class) +@SingleIn(AppScope::class) class DefaultPushService @Inject constructor( private val testPush: TestPush, private val userPushStoreFactory: UserPushStoreFactory, private val pushProviders: Set<@JvmSuppressWildcards PushProvider>, private val getCurrentPushProvider: GetCurrentPushProvider, -) : PushService { + private val sessionObserver: SessionObserver, + private val pushClientSecretStore: PushClientSecretStore, +) : PushService, SessionListener { + init { + observeSessions() + } + override suspend fun getCurrentPushProvider(): PushProvider? { val currentPushProvider = getCurrentPushProvider.getCurrentPushProvider() return pushProviders.find { it.name == currentPushProvider } @@ -47,7 +58,7 @@ class DefaultPushService @Inject constructor( val userPushStore = userPushStoreFactory.getOrCreate(matrixClient.sessionId) val currentPushProviderName = userPushStore.getPushProviderName() val currentPushProvider = pushProviders.find { it.name == currentPushProviderName } - val currentDistributorValue = currentPushProvider?.getCurrentDistributor(matrixClient)?.value + val currentDistributorValue = currentPushProvider?.getCurrentDistributor(matrixClient.sessionId)?.value if (currentPushProviderName != pushProvider.name || currentDistributorValue != distributor.value) { // Unregister previous one if any currentPushProvider @@ -65,11 +76,11 @@ class DefaultPushService @Inject constructor( } override suspend fun selectPushProvider( - matrixClient: MatrixClient, + sessionId: SessionId, pushProvider: PushProvider, ) { Timber.d("Select ${pushProvider.name}") - val userPushStore = userPushStoreFactory.getOrCreate(matrixClient.sessionId) + val userPushStore = userPushStoreFactory.getOrCreate(sessionId) userPushStore.setPushProviderName(pushProvider.name) } @@ -87,4 +98,31 @@ class DefaultPushService @Inject constructor( testPush.execute(config) return true } + + private fun observeSessions() { + sessionObserver.addListener(this) + } + + override suspend fun onSessionCreated(userId: String) { + // Nothing to do + } + + /** + * The session has been deleted. + * In this case, this is not necessary to unregister the pusher from the homeserver, + * but we need to do some cleanup locally. + * The current push provider may want to take action, and we need to + * cleanup the stores. + */ + override suspend fun onSessionDeleted(userId: String) { + val sessionId = SessionId(userId) + val userPushStore = userPushStoreFactory.getOrCreate(sessionId) + val currentPushProviderName = userPushStore.getPushProviderName() + val currentPushProvider = pushProviders.find { it.name == currentPushProviderName } + // Cleanup the current push provider. They may need the client secret, so delete the secret after. + currentPushProvider?.onSessionDeleted(sessionId) + // Now we can safely reset the stores. + pushClientSecretStore.resetSecret(sessionId) + userPushStore.reset() + } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt index f2b029da49..af638d084b 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt @@ -9,6 +9,7 @@ package io.element.android.libraries.push.impl import com.google.common.truth.Truth.assertThat import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.test.AN_EXCEPTION import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.FakeMatrixClient @@ -22,8 +23,12 @@ import io.element.android.libraries.pushproviders.api.PushProvider import io.element.android.libraries.pushproviders.test.FakePushProvider import io.element.android.libraries.pushproviders.test.aCurrentUserPushConfig import io.element.android.libraries.pushstore.api.UserPushStoreFactory +import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretStore import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushStore import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushStoreFactory +import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.InMemoryPushClientSecretStore +import io.element.android.libraries.sessionstorage.api.observer.SessionObserver +import io.element.android.libraries.sessionstorage.test.observer.NoOpSessionObserver import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value import kotlinx.coroutines.flow.first @@ -210,17 +215,87 @@ class DefaultPushServiceTest { assertThat(defaultPushService.ignoreRegistrationError(A_SESSION_ID).first()).isTrue() } + @Test + fun `onSessionCreated is noop`() = runTest { + val defaultPushService = createDefaultPushService() + defaultPushService.onSessionCreated(A_SESSION_ID.value) + } + + @Test + fun `onSessionDeleted should transmit the info to the current push provider and cleanup the stores`() = runTest { + val onSessionDeletedLambda = lambdaRecorder { } + val aCurrentPushProvider = FakePushProvider( + name = "aCurrentPushProvider", + onSessionDeletedLambda = onSessionDeletedLambda, + ) + val userPushStore = FakeUserPushStore( + pushProviderName = aCurrentPushProvider.name, + ) + val pushClientSecretStore = InMemoryPushClientSecretStore() + val defaultPushService = createDefaultPushService( + pushProviders = setOf(aCurrentPushProvider), + getCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider = aCurrentPushProvider.name), + userPushStoreFactory = FakeUserPushStoreFactory( + userPushStore = { userPushStore }, + ), + pushClientSecretStore = pushClientSecretStore, + ) + defaultPushService.onSessionDeleted(A_SESSION_ID.value) + assertThat(userPushStore.getPushProviderName()).isNull() + assertThat(pushClientSecretStore.getSecret(A_SESSION_ID)).isNull() + onSessionDeletedLambda.assertions().isCalledOnce().with(value(A_SESSION_ID)) + } + + @Test + fun `onSessionDeleted when there is no push provider should just cleanup the stores`() = runTest { + val userPushStore = FakeUserPushStore( + pushProviderName = null, + ) + val pushClientSecretStore = InMemoryPushClientSecretStore() + val defaultPushService = createDefaultPushService( + pushProviders = emptySet(), + getCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider = null), + userPushStoreFactory = FakeUserPushStoreFactory( + userPushStore = { userPushStore }, + ), + pushClientSecretStore = pushClientSecretStore, + ) + defaultPushService.onSessionDeleted(A_SESSION_ID.value) + assertThat(userPushStore.getPushProviderName()).isNull() + assertThat(pushClientSecretStore.getSecret(A_SESSION_ID)).isNull() + } + + @Test + fun `selectPushProvider should store the data in the store`() = runTest { + val userPushStore = FakeUserPushStore() + val defaultPushService = createDefaultPushService( + userPushStoreFactory = FakeUserPushStoreFactory( + userPushStore = { userPushStore }, + ), + ) + val aPushProvider = FakePushProvider( + name = "aCurrentPushProvider", + ) + assertThat(userPushStore.getPushProviderName()).isNull() + defaultPushService.selectPushProvider(A_SESSION_ID, aPushProvider) + assertThat(userPushStore.getPushProviderName()).isEqualTo(aPushProvider.name) + } + private fun createDefaultPushService( testPush: TestPush = FakeTestPush(), userPushStoreFactory: UserPushStoreFactory = FakeUserPushStoreFactory(), pushProviders: Set<@JvmSuppressWildcards PushProvider> = emptySet(), getCurrentPushProvider: GetCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider = null), + sessionObserver: SessionObserver = NoOpSessionObserver(), + pushClientSecretStore: PushClientSecretStore = InMemoryPushClientSecretStore(), ): DefaultPushService { return DefaultPushService( testPush = testPush, userPushStoreFactory = userPushStoreFactory, pushProviders = pushProviders, getCurrentPushProvider = getCurrentPushProvider, + sessionObserver = sessionObserver, + pushClientSecretStore = pushClientSecretStore, ) } } diff --git a/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt b/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt index 7af2b73fe5..6b7512afa8 100644 --- a/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt +++ b/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt @@ -24,7 +24,7 @@ class FakePushService( Result.success(Unit) }, private val currentPushProvider: () -> PushProvider? = { availablePushProviders.firstOrNull() }, - private val selectPushProviderLambda: suspend (MatrixClient, PushProvider) -> Unit = { _, _ -> lambdaError() }, + private val selectPushProviderLambda: suspend (SessionId, PushProvider) -> Unit = { _, _ -> lambdaError() }, private val setIgnoreRegistrationErrorLambda: (SessionId, Boolean) -> Unit = { _, _ -> lambdaError() }, ) : PushService { override suspend fun getCurrentPushProvider(): PushProvider? { @@ -50,8 +50,8 @@ class FakePushService( } } - override suspend fun selectPushProvider(matrixClient: MatrixClient, pushProvider: PushProvider) { - selectPushProviderLambda(matrixClient, pushProvider) + override suspend fun selectPushProvider(sessionId: SessionId, pushProvider: PushProvider) { + selectPushProviderLambda(sessionId, pushProvider) } private val ignoreRegistrationError = MutableStateFlow(false) diff --git a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt index 90b4cb0465..3c82113798 100644 --- a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt +++ b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt @@ -8,6 +8,7 @@ package io.element.android.libraries.pushproviders.api import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.SessionId /** * This is the main API for this module. @@ -36,13 +37,18 @@ interface PushProvider { /** * Return the current distributor, or null if none. */ - suspend fun getCurrentDistributor(matrixClient: MatrixClient): Distributor? + suspend fun getCurrentDistributor(sessionId: SessionId): Distributor? /** * Unregister the pusher. */ suspend fun unregister(matrixClient: MatrixClient): Result + /** + * To invoke when the session is deleted. + */ + suspend fun onSessionDeleted(sessionId: SessionId) + suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? fun canRotateToken(): Boolean diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt index b1e31a2303..9b384a6e65 100644 --- a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt @@ -11,6 +11,7 @@ import com.squareup.anvil.annotations.ContributesMultibinding import io.element.android.libraries.core.log.logger.LoggerTag import io.element.android.libraries.di.AppScope import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushproviders.api.CurrentUserPushConfig import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider @@ -51,7 +52,7 @@ class FirebasePushProvider @Inject constructor( ) } - override suspend fun getCurrentDistributor(matrixClient: MatrixClient) = firebaseDistributor + override suspend fun getCurrentDistributor(sessionId: SessionId) = firebaseDistributor override suspend fun unregister(matrixClient: MatrixClient): Result { val pushKey = firebaseStore.getFcmToken() @@ -63,6 +64,11 @@ class FirebasePushProvider @Inject constructor( } } + /** + * Nothing to clean up here. + */ + override suspend fun onSessionDeleted(sessionId: SessionId) = Unit + override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { return firebaseStore.getFcmToken()?.let { fcmToken -> CurrentUserPushConfig( diff --git a/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt index 6a3a9474e4..b5bac64b09 100644 --- a/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt +++ b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt @@ -10,6 +10,7 @@ package io.element.android.libraries.pushproviders.firebase import com.google.common.truth.Truth.assertThat import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.test.AN_EXCEPTION +import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.push.test.FakePusherSubscriber import io.element.android.libraries.pushproviders.api.CurrentUserPushConfig @@ -47,9 +48,9 @@ class FirebasePushProviderTest { } @Test - fun `getCurrentDistributor always return the unique distributor`() = runTest { + fun `getCurrentDistributor always returns the unique distributor`() = runTest { val firebasePushProvider = createFirebasePushProvider() - val result = firebasePushProvider.getCurrentDistributor(FakeMatrixClient()) + val result = firebasePushProvider.getCurrentDistributor(A_SESSION_ID) assertThat(result).isEqualTo(Distributor("Firebase", "Firebase")) } @@ -176,6 +177,18 @@ class FirebasePushProviderTest { lambda.assertions().isCalledOnce() } + @Test + fun `canRotateToken should return true`() = runTest { + val firebasePushProvider = createFirebasePushProvider() + assertThat(firebasePushProvider.canRotateToken()).isTrue() + } + + @Test + fun `onSessionDeleted should be noop`() = runTest { + val firebasePushProvider = createFirebasePushProvider() + firebasePushProvider.onSessionDeleted(A_SESSION_ID) + } + private fun createFirebasePushProvider( firebaseStore: FirebaseStore = InMemoryFirebaseStore(), pusherSubscriber: PusherSubscriber = FakePusherSubscriber(), diff --git a/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt b/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt index 6f5bdda5b7..4d8006b9e3 100644 --- a/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt +++ b/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt @@ -8,6 +8,7 @@ package io.element.android.libraries.pushproviders.test import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushproviders.api.CurrentUserPushConfig import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider @@ -21,6 +22,7 @@ class FakePushProvider( private val currentUserPushConfig: CurrentUserPushConfig? = null, private val registerWithResult: (MatrixClient, Distributor) -> Result = { _, _ -> lambdaError() }, private val unregisterWithResult: (MatrixClient) -> Result = { lambdaError() }, + private val onSessionDeletedLambda: (SessionId) -> Unit = { lambdaError() }, private val canRotateTokenResult: () -> Boolean = { lambdaError() }, private val rotateTokenLambda: () -> Result = { lambdaError() }, ) : PushProvider { @@ -30,7 +32,7 @@ class FakePushProvider( return registerWithResult(matrixClient, distributor) } - override suspend fun getCurrentDistributor(matrixClient: MatrixClient): Distributor? { + override suspend fun getCurrentDistributor(sessionId: SessionId): Distributor? { return currentDistributor() } @@ -38,6 +40,10 @@ class FakePushProvider( return unregisterWithResult(matrixClient) } + override suspend fun onSessionDeleted(sessionId: SessionId) { + onSessionDeletedLambda(sessionId) + } + override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { return currentUserPushConfig } diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt index 1ce929a75b..b598d3371e 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt @@ -10,6 +10,7 @@ package io.element.android.libraries.pushproviders.unifiedpush import com.squareup.anvil.annotations.ContributesMultibinding import io.element.android.libraries.di.AppScope import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushproviders.api.CurrentUserPushConfig import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider @@ -40,14 +41,19 @@ class UnifiedPushProvider @Inject constructor( } } - override suspend fun getCurrentDistributor(matrixClient: MatrixClient): Distributor? { - val distributorValue = unifiedPushStore.getDistributorValue(matrixClient.sessionId) + override suspend fun getCurrentDistributor(sessionId: SessionId): Distributor? { + val distributorValue = unifiedPushStore.getDistributorValue(sessionId) return getDistributors().find { it.value == distributorValue } } override suspend fun unregister(matrixClient: MatrixClient): Result { val clientSecret = pushClientSecret.getSecretForUser(matrixClient.sessionId) - return unRegisterUnifiedPushUseCase.execute(matrixClient, clientSecret) + return unRegisterUnifiedPushUseCase.unregister(matrixClient, clientSecret) + } + + override suspend fun onSessionDeleted(sessionId: SessionId) { + val clientSecret = pushClientSecret.getSecretForUser(sessionId) + unRegisterUnifiedPushUseCase.cleanup(clientSecret) } override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt index f379b97726..2c2613901c 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt @@ -18,7 +18,15 @@ import timber.log.Timber import javax.inject.Inject interface UnregisterUnifiedPushUseCase { - suspend fun execute(matrixClient: MatrixClient, clientSecret: String): Result + /** + * Unregister the app from the homeserver, then from UnifiedPush. + */ + suspend fun unregister(matrixClient: MatrixClient, clientSecret: String): Result + + /** + * Cleanup any remaining data for the given client secret and unregister the app from UnifiedPush. + */ + fun cleanup(clientSecret: String) } @ContributesBinding(AppScope::class) @@ -27,21 +35,24 @@ class DefaultUnregisterUnifiedPushUseCase @Inject constructor( private val unifiedPushStore: UnifiedPushStore, private val pusherSubscriber: PusherSubscriber, ) : UnregisterUnifiedPushUseCase { - override suspend fun execute(matrixClient: MatrixClient, clientSecret: String): Result { + override suspend fun unregister(matrixClient: MatrixClient, clientSecret: String): Result { val endpoint = unifiedPushStore.getEndpoint(clientSecret) val gateway = unifiedPushStore.getPushGateway(clientSecret) if (endpoint == null || gateway == null) { Timber.w("No endpoint or gateway found for client secret") // Ensure we don't have any remaining data, but ignore this error - unifiedPushStore.storeUpEndpoint(clientSecret, null) - unifiedPushStore.storePushGateway(clientSecret, null) + cleanup(clientSecret) return Result.success(Unit) } return pusherSubscriber.unregisterPusher(matrixClient, endpoint, gateway) .onSuccess { - unifiedPushStore.storeUpEndpoint(clientSecret, null) - unifiedPushStore.storePushGateway(clientSecret, null) - UnifiedPush.unregisterApp(context) + cleanup(clientSecret) } } + + override fun cleanup(clientSecret: String) { + unifiedPushStore.storeUpEndpoint(clientSecret, null) + unifiedPushStore.storePushGateway(clientSecret, null) + UnifiedPush.unregisterApp(context, clientSecret) + } } diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt index 24f95d94c2..0c05748b03 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt @@ -41,7 +41,7 @@ class DefaultUnregisterUnifiedPushUseCaseTest { unregisterPusherResult = lambda ) ) - val result = useCase.execute(matrixClient, A_SECRET) + val result = useCase.unregister(matrixClient, A_SECRET) assertThat(result.isSuccess).isTrue() lambda.assertions() .isCalledOnce() @@ -67,7 +67,7 @@ class DefaultUnregisterUnifiedPushUseCaseTest { storePushGatewayResult = storePushGatewayResult, ), ) - val result = useCase.execute(matrixClient, A_SECRET) + val result = useCase.unregister(matrixClient, A_SECRET) assertThat(result.isSuccess).isTrue() storeUpEndpointResult.assertions() .isCalledOnce() @@ -90,7 +90,7 @@ class DefaultUnregisterUnifiedPushUseCaseTest { storePushGatewayResult = storePushGatewayResult, ), ) - val result = useCase.execute(matrixClient, A_SECRET) + val result = useCase.unregister(matrixClient, A_SECRET) assertThat(result.isSuccess).isTrue() storeUpEndpointResult.assertions() .isCalledOnce() @@ -112,7 +112,7 @@ class DefaultUnregisterUnifiedPushUseCaseTest { unregisterPusherResult = { _, _, _ -> Result.failure(AN_EXCEPTION) } ) ) - val result = useCase.execute(matrixClient, A_SECRET) + val result = useCase.unregister(matrixClient, A_SECRET) assertThat(result.isFailure).isTrue() } diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt index 145681de8c..d04ef25bfb 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt @@ -11,9 +11,14 @@ import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.tests.testutils.lambda.lambdaError class FakeUnregisterUnifiedPushUseCase( - private val result: (MatrixClient, String) -> Result = { _, _ -> lambdaError() } + private val unregisterLambda: (MatrixClient, String) -> Result = { _, _ -> lambdaError() }, + private val cleanupLambda: (String) -> Unit = { lambdaError() }, ) : UnregisterUnifiedPushUseCase { - override suspend fun execute(matrixClient: MatrixClient, clientSecret: String): Result { - return result(matrixClient, clientSecret) + override suspend fun unregister(matrixClient: MatrixClient, clientSecret: String): Result { + return unregisterLambda(matrixClient, clientSecret) + } + + override fun cleanup(clientSecret: String) { + cleanupLambda(clientSecret) } } diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt index 93e6a106e9..8282cbcbe6 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt @@ -117,13 +117,13 @@ class UnifiedPushProviderTest { fun `unregister ok`() = runTest { val matrixClient = FakeMatrixClient() val getSecretForUserResultLambda = lambdaRecorder { A_SECRET } - val executeLambda = lambdaRecorder> { _, _ -> Result.success(Unit) } + val unregisterLambda = lambdaRecorder> { _, _ -> Result.success(Unit) } val unifiedPushProvider = createUnifiedPushProvider( pushClientSecret = FakePushClientSecret( getSecretForUserResult = getSecretForUserResultLambda, ), unRegisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( - result = executeLambda, + unregisterLambda = unregisterLambda, ), ) val result = unifiedPushProvider.unregister(matrixClient) @@ -131,7 +131,7 @@ class UnifiedPushProviderTest { getSecretForUserResultLambda.assertions() .isCalledOnce() .with(value(A_SESSION_ID)) - executeLambda.assertions() + unregisterLambda.assertions() .isCalledOnce() .with(value(matrixClient), value(A_SECRET)) } @@ -140,13 +140,13 @@ class UnifiedPushProviderTest { fun `unregister ko`() = runTest { val matrixClient = FakeMatrixClient() val getSecretForUserResultLambda = lambdaRecorder { A_SECRET } - val executeLambda = lambdaRecorder> { _, _ -> Result.failure(AN_EXCEPTION) } + val unregisterLambda = lambdaRecorder> { _, _ -> Result.failure(AN_EXCEPTION) } val unifiedPushProvider = createUnifiedPushProvider( pushClientSecret = FakePushClientSecret( getSecretForUserResult = getSecretForUserResultLambda, ), unRegisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( - result = executeLambda, + unregisterLambda = unregisterLambda, ), ) val result = unifiedPushProvider.unregister(matrixClient) @@ -154,7 +154,7 @@ class UnifiedPushProviderTest { getSecretForUserResultLambda.assertions() .isCalledOnce() .with(value(A_SESSION_ID)) - executeLambda.assertions() + unregisterLambda.assertions() .isCalledOnce() .with(value(matrixClient), value(A_SECRET)) } @@ -162,7 +162,6 @@ class UnifiedPushProviderTest { @Test fun `getCurrentDistributor ok`() = runTest { val distributor = Distributor("value", "Name") - val matrixClient = FakeMatrixClient() val unifiedPushProvider = createUnifiedPushProvider( unifiedPushStore = FakeUnifiedPushStore( getDistributorValueResult = { distributor.value } @@ -174,14 +173,13 @@ class UnifiedPushProviderTest { ) ) ) - val result = unifiedPushProvider.getCurrentDistributor(matrixClient) + val result = unifiedPushProvider.getCurrentDistributor(A_SESSION_ID) assertThat(result).isEqualTo(distributor) } @Test fun `getCurrentDistributor not know`() = runTest { val distributor = Distributor("value", "Name") - val matrixClient = FakeMatrixClient() val unifiedPushProvider = createUnifiedPushProvider( unifiedPushStore = FakeUnifiedPushStore( getDistributorValueResult = { "unknown" } @@ -192,14 +190,13 @@ class UnifiedPushProviderTest { ) ) ) - val result = unifiedPushProvider.getCurrentDistributor(matrixClient) + val result = unifiedPushProvider.getCurrentDistributor(A_SESSION_ID) assertThat(result).isNull() } @Test fun `getCurrentDistributor not found`() = runTest { val distributor = Distributor("value", "Name") - val matrixClient = FakeMatrixClient() val unifiedPushProvider = createUnifiedPushProvider( unifiedPushStore = FakeUnifiedPushStore( getDistributorValueResult = { distributor.value } @@ -208,7 +205,7 @@ class UnifiedPushProviderTest { getDistributorsResult = emptyList() ) ) - val result = unifiedPushProvider.getCurrentDistributor(matrixClient) + val result = unifiedPushProvider.getCurrentDistributor(A_SESSION_ID) assertThat(result).isNull() } @@ -224,6 +221,27 @@ class UnifiedPushProviderTest { assertThat(result).isEqualTo(currentUserPushConfig) } + @Test + fun `canRotateToken should return false`() = runTest { + val unifiedPushProvider = createUnifiedPushProvider() + assertThat(unifiedPushProvider.canRotateToken()).isFalse() + } + + @Test + fun `onSessionDeleted should do the cleanup`() = runTest { + val cleanupLambda = lambdaRecorder { } + val unifiedPushProvider = createUnifiedPushProvider( + pushClientSecret = FakePushClientSecret( + getSecretForUserResult = { A_SECRET } + ), + unRegisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( + cleanupLambda = cleanupLambda, + ), + ) + unifiedPushProvider.onSessionDeleted(A_SESSION_ID) + cleanupLambda.assertions().isCalledOnce().with(value(A_SECRET)) + } + private fun createUnifiedPushProvider( unifiedPushDistributorProvider: UnifiedPushDistributorProvider = FakeUnifiedPushDistributorProvider(), registerUnifiedPushUseCase: RegisterUnifiedPushUseCase = FakeRegisterUnifiedPushUseCase(), diff --git a/libraries/pushstore/impl/build.gradle.kts b/libraries/pushstore/impl/build.gradle.kts index cca5f3445a..c4eafd896c 100644 --- a/libraries/pushstore/impl/build.gradle.kts +++ b/libraries/pushstore/impl/build.gradle.kts @@ -28,7 +28,6 @@ dependencies { implementation(projects.libraries.core) implementation(projects.libraries.matrix.api) implementation(projects.libraries.pushstore.api) - implementation(projects.libraries.sessionStorage.api) implementation(libs.androidx.corektx) implementation(libs.androidx.datastore.preferences) @@ -41,7 +40,6 @@ dependencies { testImplementation(projects.libraries.matrix.test) testImplementation(projects.services.appnavstate.test) testImplementation(projects.libraries.pushstore.test) - testImplementation(projects.libraries.sessionStorage.test) androidTestImplementation(libs.coroutines.test) androidTestImplementation(libs.test.core) diff --git a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt index 714d8bece7..f4cb77f01e 100644 --- a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt +++ b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt @@ -15,21 +15,14 @@ import io.element.android.libraries.di.SingleIn import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushstore.api.UserPushStore import io.element.android.libraries.pushstore.api.UserPushStoreFactory -import io.element.android.libraries.sessionstorage.api.observer.SessionListener -import io.element.android.libraries.sessionstorage.api.observer.SessionObserver import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject @SingleIn(AppScope::class) -@ContributesBinding(AppScope::class, boundType = UserPushStoreFactory::class) +@ContributesBinding(AppScope::class) class DefaultUserPushStoreFactory @Inject constructor( @ApplicationContext private val context: Context, - private val sessionObserver: SessionObserver, -) : UserPushStoreFactory, SessionListener { - init { - observeSessions() - } - +) : UserPushStoreFactory { // We can have only one class accessing a single data store, so keep a cache of them. private val cache = ConcurrentHashMap() override fun getOrCreate(userId: SessionId): UserPushStore { @@ -40,17 +33,4 @@ class DefaultUserPushStoreFactory @Inject constructor( ) } } - - private fun observeSessions() { - sessionObserver.addListener(this) - } - - override suspend fun onSessionCreated(userId: String) { - // Nothing to do - } - - override suspend fun onSessionDeleted(userId: String) { - // Delete the store - getOrCreate(SessionId(userId)).reset() - } } diff --git a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecret.kt b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecret.kt index c4404c7c21..d488b69213 100644 --- a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecret.kt +++ b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecret.kt @@ -9,26 +9,17 @@ package io.element.android.libraries.pushstore.impl.clientsecret import com.squareup.anvil.annotations.ContributesBinding import io.element.android.libraries.di.AppScope -import io.element.android.libraries.di.SingleIn import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretFactory import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretStore -import io.element.android.libraries.sessionstorage.api.observer.SessionListener -import io.element.android.libraries.sessionstorage.api.observer.SessionObserver import javax.inject.Inject -@SingleIn(AppScope::class) -@ContributesBinding(AppScope::class, boundType = PushClientSecret::class) +@ContributesBinding(AppScope::class) class DefaultPushClientSecret @Inject constructor( private val pushClientSecretFactory: PushClientSecretFactory, private val pushClientSecretStore: PushClientSecretStore, - private val sessionObserver: SessionObserver, -) : PushClientSecret, SessionListener { - init { - observeSessions() - } - +) : PushClientSecret { override suspend fun getSecretForUser(userId: SessionId): String { val existingSecret = pushClientSecretStore.getSecret(userId) if (existingSecret != null) { @@ -42,17 +33,4 @@ class DefaultPushClientSecret @Inject constructor( override suspend fun getUserIdFromSecret(clientSecret: String): SessionId? { return pushClientSecretStore.getUserIdFromSecret(clientSecret) } - - private fun observeSessions() { - sessionObserver.addListener(this) - } - - override suspend fun onSessionCreated(userId: String) { - // Nothing to do - } - - override suspend fun onSessionDeleted(userId: String) { - // Delete the secret - pushClientSecretStore.resetSecret(SessionId(userId)) - } } diff --git a/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecretTest.kt b/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecretTest.kt index 6c02ec2eb0..f4569468d7 100644 --- a/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecretTest.kt +++ b/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecretTest.kt @@ -10,7 +10,6 @@ package io.element.android.libraries.pushstore.impl.clientsecret import com.google.common.truth.Truth.assertThat import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.InMemoryPushClientSecretStore -import io.element.android.libraries.sessionstorage.test.observer.NoOpSessionObserver import kotlinx.coroutines.test.runTest import org.junit.Test @@ -24,11 +23,10 @@ internal class DefaultPushClientSecretTest { fun test() = runTest { val factory = FakePushClientSecretFactory() val store = InMemoryPushClientSecretStore() - val sut = DefaultPushClientSecret(factory, store, NoOpSessionObserver()) + val sut = DefaultPushClientSecret(factory, store) val secret0 = factory.getSecretForUser(0) val secret1 = factory.getSecretForUser(1) - val secret2 = factory.getSecretForUser(2) assertThat(store.getSecrets()).isEmpty() assertThat(sut.getUserIdFromSecret(secret0)).isNull() @@ -48,16 +46,10 @@ internal class DefaultPushClientSecretTest { // Unknown secret assertThat(sut.getUserIdFromSecret(A_UNKNOWN_SECRET)).isNull() - // User signs out - sut.onSessionDeleted(A_USER_ID_0.value) - assertThat(store.getSecrets()).hasSize(1) - // Create a new secret after reset - assertThat(sut.getSecretForUser(A_USER_ID_0)).isEqualTo(secret2) - // Check the store content assertThat(store.getSecrets()).isEqualTo( mapOf( - A_USER_ID_0 to secret2, + A_USER_ID_0 to secret0, A_USER_ID_1 to secret1, ) ) diff --git a/libraries/pushstore/test/src/main/kotlin/io/element/android/libraries/pushstore/test/userpushstore/FakeUserPushStore.kt b/libraries/pushstore/test/src/main/kotlin/io/element/android/libraries/pushstore/test/userpushstore/FakeUserPushStore.kt index 50507fea00..9c616c1c4b 100644 --- a/libraries/pushstore/test/src/main/kotlin/io/element/android/libraries/pushstore/test/userpushstore/FakeUserPushStore.kt +++ b/libraries/pushstore/test/src/main/kotlin/io/element/android/libraries/pushstore/test/userpushstore/FakeUserPushStore.kt @@ -54,5 +54,6 @@ class FakeUserPushStore( } override suspend fun reset() { + pushProviderName = null } }