fix: missing ServerConfig crashes after session expired / logout [WPB…

…-5960] (#2352) (#2354) * fix: missing ServerConfig crashes after session expired / logout [WPB-5960] * simplify LogoutCallback * make LogoutCallback suspend * changes after review * make LogoutCallback to become interface to fix tests Co-authored-by: Michał Saleniuk <[email protected]> Co-authored-by: Oussama Hassine <[email protected]>
wireapp · Jan 10, 2024 · 46924bb · 46924bb
1 parent 52f2033
commit 46924bb
Show file tree

Hide file tree

Showing 18 changed files with 133 additions and 16 deletions.
diff --git a/logic/src/androidMain/kotlin/com/wire/kalium/logic/CoreLogic.kt b/logic/src/androidMain/kotlin/com/wire/kalium/logic/CoreLogic.kt
@@ -88,6 +88,7 @@ actual class CoreLogic(
             globalCallManager,
             userStorageProvider,
             networkStateObserver,
+            logoutCallbackManager,
             userAgent
         )
     }

diff --git a/logic/src/androidMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt b/logic/src/androidMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt
@@ -29,6 +29,7 @@ import com.wire.kalium.logic.di.PlatformUserStorageProperties
 import com.wire.kalium.logic.di.RootPathsProvider
 import com.wire.kalium.logic.di.UserStorageProvider
 import com.wire.kalium.logic.feature.auth.AuthenticationScopeProvider
+import com.wire.kalium.logic.feature.auth.LogoutCallback
 import com.wire.kalium.logic.feature.call.GlobalCallManager
 import com.wire.kalium.logic.featureFlags.KaliumConfigs
 import com.wire.kalium.network.NetworkStateObserver
@@ -52,6 +53,7 @@ internal fun UserSessionScope(
     userStorageProvider: UserStorageProvider,
     userSessionScopeProvider: UserSessionScopeProvider,
     networkStateObserver: NetworkStateObserver,
+    logoutCallback: LogoutCallback,
 ): UserSessionScope {
     val platformUserStorageProperties =
         PlatformUserStorageProperties(applicationContext, SecurityHelperImpl(globalPreferences.passphraseStorage))
@@ -73,6 +75,7 @@ internal fun UserSessionScope(
         userStorageProvider,
         clientConfig,
         platformUserStorageProperties,
-        networkStateObserver
+        networkStateObserver,
+        logoutCallback
     )
 }
diff --git a/logic/src/androidMain/kotlin/com/wire/kalium/logic/feature/UserSessionScopeProviderImpl.kt b/logic/src/androidMain/kotlin/com/wire/kalium/logic/feature/UserSessionScopeProviderImpl.kt
@@ -30,6 +30,7 @@ import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.di.RootPathsProvider
 import com.wire.kalium.logic.di.UserStorageProvider
 import com.wire.kalium.logic.feature.auth.AuthenticationScopeProvider
+import com.wire.kalium.logic.feature.auth.LogoutCallback
 import com.wire.kalium.logic.feature.call.GlobalCallManager
 import com.wire.kalium.logic.featureFlags.KaliumConfigs
 import com.wire.kalium.network.NetworkStateObserver
@@ -48,6 +49,7 @@ internal actual class UserSessionScopeProviderImpl(
     private val globalCallManager: GlobalCallManager,
     private val userStorageProvider: UserStorageProvider,
     private val networkStateObserver: NetworkStateObserver,
+    private val logoutCallback: LogoutCallback,
     userAgent: String
 ) : UserSessionScopeProviderCommon(globalCallManager, userStorageProvider, userAgent) {
 
@@ -73,6 +75,7 @@ internal actual class UserSessionScopeProviderImpl(
             userStorageProvider = userStorageProvider,
             userSessionScopeProvider = this,
             networkStateObserver = networkStateObserver,
+            logoutCallback = logoutCallback,
         )
     }
 }
diff --git a/logic/src/appleMain/kotlin/com/wire/kalium/logic/CoreLogic.kt b/logic/src/appleMain/kotlin/com/wire/kalium/logic/CoreLogic.kt
@@ -59,6 +59,7 @@ actual class CoreLogic(
             globalCallManager,
             userStorageProvider,
             networkStateObserver,
+            logoutCallbackManager,
             userAgent
         )
     }

diff --git a/logic/src/appleMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt b/logic/src/appleMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt
@@ -28,6 +28,7 @@ import com.wire.kalium.logic.di.PlatformUserStorageProperties
 import com.wire.kalium.logic.di.RootPathsProvider
 import com.wire.kalium.logic.di.UserStorageProvider
 import com.wire.kalium.logic.feature.auth.AuthenticationScopeProvider
+import com.wire.kalium.logic.feature.auth.LogoutCallback
 import com.wire.kalium.logic.feature.call.GlobalCallManager
 import com.wire.kalium.logic.featureFlags.KaliumConfigs
 import com.wire.kalium.network.NetworkStateObserver
@@ -49,6 +50,7 @@ internal fun UserSessionScope(
     userStorageProvider: UserStorageProvider,
     userSessionScopeProvider: UserSessionScopeProvider,
     networkStateObserver: NetworkStateObserver,
+    logoutCallback: LogoutCallback,
     userAgent: String
 ): UserSessionScope {
 
@@ -69,6 +71,7 @@ internal fun UserSessionScope(
         userStorageProvider,
         clientConfig,
         platformUserStorageProperties,
-        networkStateObserver
+        networkStateObserver,
+        logoutCallback
     )
 }
diff --git a/logic/src/appleMain/kotlin/com/wire/kalium/logic/feature/UserSessionScopeProviderImpl.kt b/logic/src/appleMain/kotlin/com/wire/kalium/logic/feature/UserSessionScopeProviderImpl.kt
@@ -29,6 +29,7 @@ import com.wire.kalium.logic.di.PlatformUserStorageProperties
 import com.wire.kalium.logic.di.RootPathsProvider
 import com.wire.kalium.logic.di.UserStorageProvider
 import com.wire.kalium.logic.feature.auth.AuthenticationScopeProvider
+import com.wire.kalium.logic.feature.auth.LogoutCallback
 import com.wire.kalium.logic.feature.call.GlobalCallManager
 import com.wire.kalium.logic.featureFlags.KaliumConfigs
 import com.wire.kalium.network.NetworkStateObserver
@@ -45,6 +46,7 @@ internal actual class UserSessionScopeProviderImpl(
     private val globalCallManager: GlobalCallManager,
     private val userStorageProvider: UserStorageProvider,
     private val networkStateObserver: NetworkStateObserver,
+    private val logoutCallback: LogoutCallback,
     userAgent: String
 ) : UserSessionScopeProviderCommon(globalCallManager, userStorageProvider, userAgent) {
     override fun create(userId: UserId): UserSessionScope {
@@ -69,6 +71,7 @@ internal actual class UserSessionScopeProviderImpl(
             userStorageProvider,
             this,
             networkStateObserver,
+            logoutCallback,
             userAgent
             )
     }

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreLogic.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreLogic.kt
@@ -31,6 +31,7 @@ import com.wire.kalium.logic.feature.UserSessionScope
 import com.wire.kalium.logic.feature.UserSessionScopeProvider
 import com.wire.kalium.logic.feature.auth.AuthenticationScope
 import com.wire.kalium.logic.feature.auth.AuthenticationScopeProvider
+import com.wire.kalium.logic.feature.auth.LogoutCallbackManagerImpl
 import com.wire.kalium.logic.feature.auth.autoVersioningAuth.AutoVersionAuthScopeUseCase
 import com.wire.kalium.logic.feature.call.GlobalCallManager
 import com.wire.kalium.logic.featureFlags.KaliumConfigs
@@ -57,16 +58,19 @@ abstract class CoreLogicCommon internal constructor(
     protected val authenticationScopeProvider: AuthenticationScopeProvider =
         AuthenticationScopeProvider(userAgent)
 
-    fun getGlobalScope(): GlobalKaliumScope =
+    private val globalKaliumScope by lazy {
         GlobalKaliumScope(
             userAgent,
             globalDatabase,
             globalPreferences,
             kaliumConfigs,
             userSessionScopeProvider,
             authenticationScopeProvider,
-            networkStateObserver
+            networkStateObserver,
+            logoutCallbackManager,
         )
+    }
+    fun getGlobalScope(): GlobalKaliumScope = globalKaliumScope
 
     @Suppress("MemberVisibilityCanBePrivate") // Can be used by other targets like iOS and JS
     fun getAuthenticationScope(
@@ -108,6 +112,8 @@ abstract class CoreLogicCommon internal constructor(
         AutoVersionAuthScopeUseCase(kaliumConfigs, serverLinks, this)
 
     abstract val networkStateObserver: NetworkStateObserver
+
+    internal val logoutCallbackManager = LogoutCallbackManagerImpl()
 }
 
 expect val clientPlatform: String
diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/GlobalKaliumScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/GlobalKaliumScope.kt
@@ -31,6 +31,7 @@ import com.wire.kalium.logic.feature.appVersioning.ObserveIfAppUpdateRequiredUse
 import com.wire.kalium.logic.feature.appVersioning.ObserveIfAppUpdateRequiredUseCaseImpl
 import com.wire.kalium.logic.feature.auth.AddAuthenticatedUserUseCase
 import com.wire.kalium.logic.feature.auth.AuthenticationScopeProvider
+import com.wire.kalium.logic.feature.auth.LogoutCallbackManager
 import com.wire.kalium.logic.feature.auth.ValidateEmailUseCase
 import com.wire.kalium.logic.feature.auth.ValidateEmailUseCaseImpl
 import com.wire.kalium.logic.feature.auth.ValidatePasswordUseCase
@@ -92,7 +93,8 @@ class GlobalKaliumScope internal constructor(
     private val kaliumConfigs: KaliumConfigs,
     private val userSessionScopeProvider: Lazy<UserSessionScopeProvider>,
     private val authenticationScopeProvider: AuthenticationScopeProvider,
-    private val networkStateObserver: NetworkStateObserver
+    private val networkStateObserver: NetworkStateObserver,
+    val logoutCallbackManager: LogoutCallbackManager,
 ) : CoroutineScope {
 
     override val coroutineContext: CoroutineContext = SupervisorJob()

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt
@@ -153,6 +153,7 @@ import com.wire.kalium.logic.feature.auth.AuthenticationScope
 import com.wire.kalium.logic.feature.auth.AuthenticationScopeProvider
 import com.wire.kalium.logic.feature.auth.ClearUserDataUseCase
 import com.wire.kalium.logic.feature.auth.ClearUserDataUseCaseImpl
+import com.wire.kalium.logic.feature.auth.LogoutCallback
 import com.wire.kalium.logic.feature.auth.LogoutUseCase
 import com.wire.kalium.logic.feature.auth.LogoutUseCaseImpl
 import com.wire.kalium.logic.feature.backup.BackupScope
@@ -442,7 +443,8 @@ class UserSessionScope internal constructor(
     userStorageProvider: UserStorageProvider,
     private val clientConfig: ClientConfig,
     platformUserStorageProperties: PlatformUserStorageProperties,
-    networkStateObserver: NetworkStateObserver
+    networkStateObserver: NetworkStateObserver,
+    private val logoutCallback: LogoutCallback,
 ) : CoroutineScope {
 
     private val userStorage = userStorageProvider.getOrCreate(
@@ -1696,6 +1698,7 @@ class UserSessionScope internal constructor(
             userSessionWorkScheduler,
             calls.establishedCall,
             calls.endCall,
+            logoutCallback,
             kaliumConfigs
         )
     val persistPersistentWebSocketConnectionStatus: PersistPersistentWebSocketConnectionStatusUseCase

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/auth/LogoutCallbackManager.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/auth/LogoutCallbackManager.kt
@@ -0,0 +1,45 @@
+/*
+ * Wire
+ * Copyright (C) 2024 Wire Swiss GmbH
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see http://www.gnu.org/licenses/.
+ */
+package com.wire.kalium.logic.feature.auth
+
+import co.touchlab.stately.collections.ConcurrentMutableList
+import com.wire.kalium.logic.data.logout.LogoutReason
+import com.wire.kalium.logic.data.user.UserId
+
+/**
+ * This manager is used to register callbacks that will be called when a user session is being logged out.
+ * The app may have some actions to perform outside the kalium as well when a user is logged out, like clearing the data store, etc.
+ * When logout action is triggered by the user, then the app can execute these actions right after the [LogoutUseCase] result,
+ * but when the logout is triggered automatically by some event (i.e. session expired, device removed, account removed),
+ * then the app will not be able to execute these actions without registering to this manager.
+ */
+interface LogoutCallbackManager {
+    fun register(callback: LogoutCallback)
+    fun unregister(callback: LogoutCallback)
+}
+
+internal class LogoutCallbackManagerImpl : LogoutCallbackManager, LogoutCallback {
+    private val callbacks = ConcurrentMutableList<LogoutCallback>()
+    override fun register(callback: LogoutCallback) { callbacks.add(callback) }
+    override fun unregister(callback: LogoutCallback) { callbacks.remove(callback) }
+    override suspend fun invoke(userId: UserId, reason: LogoutReason) { callbacks.forEach { it(userId, reason) } }
+}
+
+interface LogoutCallback {
+    suspend operator fun invoke(userId: UserId, reason: LogoutReason)
+}
diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/auth/LogoutUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/auth/LogoutUseCase.kt
@@ -63,6 +63,7 @@ internal class LogoutUseCaseImpl @Suppress("LongParameterList") constructor(
     private val userSessionWorkScheduler: UserSessionWorkScheduler,
     private val getEstablishedCallsUseCase: ObserveEstablishedCallsUseCase,
     private val endCallUseCase: EndCallUseCase,
+    private val logoutCallback: LogoutCallback,
     private val kaliumConfigs: KaliumConfigs
 ) : LogoutUseCase {
     // TODO(refactor): Maybe we can simplify by taking some of the responsibility away from here.
@@ -71,13 +72,12 @@ internal class LogoutUseCaseImpl @Suppress("LongParameterList") constructor(
 
     override suspend operator fun invoke(reason: LogoutReason, waitUntilCompletes: Boolean) {
         globalCoroutineScope.launch {
-            deregisterTokenUseCase()
-
             getEstablishedCallsUseCase().firstOrNull()?.forEach {
                 endCallUseCase(it.conversationId)
             }
 
             if (reason != LogoutReason.SESSION_EXPIRED) {
+                deregisterTokenUseCase()
                 logoutRepository.logout()
             }
 
@@ -108,6 +108,7 @@ internal class LogoutUseCaseImpl @Suppress("LongParameterList") constructor(
 
             userSessionScopeProvider.get(userId)?.cancel()
             userSessionScopeProvider.delete(userId)
+            logoutCallback(userId, reason)
         }.let { if (waitUntilCompletes) it.join() else it }
     }
 

diff --git a/.../src/commonMain/kotlin/com/wire/kalium/logic/feature/session/CurrentSessionFlowUseCase.kt b/.../src/commonMain/kotlin/com/wire/kalium/logic/feature/session/CurrentSessionFlowUseCase.kt
@@ -22,6 +22,7 @@ import com.wire.kalium.logic.StorageFailure
 import com.wire.kalium.logic.data.session.SessionRepository
 import com.wire.kalium.logic.functional.fold
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.map
 
 /**
@@ -40,5 +41,5 @@ class CurrentSessionFlowUseCase(private val sessionRepository: SessionRepository
             }, {
                 CurrentSessionResult.Success(it)
             })
-        }
+        }.distinctUntilChanged()
 }
diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/slow/SlowSyncCriteriaProvider.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/slow/SlowSyncCriteriaProvider.kt
@@ -44,7 +44,7 @@ internal interface SyncCriteriaResolution {
     /**
      * At least a criterion is not satisfied
      */
-    class MissingRequirement(val cause: String) : SyncCriteriaResolution
+    data class MissingRequirement(val cause: String) : SyncCriteriaResolution
 }
 
 /**

diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/auth/LogoutUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/auth/LogoutUseCaseTest.kt
@@ -22,16 +22,17 @@ package com.wire.kalium.logic.feature.auth
 import com.wire.kalium.logic.CoreFailure
 import com.wire.kalium.logic.StorageFailure
 import com.wire.kalium.logic.data.auth.AccountInfo
+import com.wire.kalium.logic.data.call.Call
 import com.wire.kalium.logic.data.client.ClientRepository
 import com.wire.kalium.logic.data.id.ConversationId
 import com.wire.kalium.logic.data.id.QualifiedID
 import com.wire.kalium.logic.data.logout.LogoutReason
 import com.wire.kalium.logic.data.logout.LogoutRepository
 import com.wire.kalium.logic.data.notification.PushTokenRepository
 import com.wire.kalium.logic.data.session.SessionRepository
+import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.feature.UserSessionScope
 import com.wire.kalium.logic.feature.UserSessionScopeProvider
-import com.wire.kalium.logic.data.call.Call
 import com.wire.kalium.logic.feature.call.usecase.EndCallUseCase
 import com.wire.kalium.logic.feature.call.usecase.ObserveEstablishedCallsUseCase
 import com.wire.kalium.logic.feature.client.ClearClientDataUseCase
@@ -79,10 +80,6 @@ class LogoutUseCaseTest {
             logoutUseCase.invoke(reason)
             arrangement.globalTestScope.advanceUntilIdle()
 
-            verify(arrangement.deregisterTokenUseCase)
-                .suspendFunction(arrangement.deregisterTokenUseCase::invoke)
-                .wasInvoked(exactly = once)
-
             verify(arrangement.sessionRepository)
                 .suspendFunction(arrangement.sessionRepository::logout)
                 .with(any(), eq(reason))
@@ -107,6 +104,10 @@ class LogoutUseCaseTest {
                     .with(eq(true))
                     .wasInvoked(exactly = once)
             }
+            verify(arrangement.logoutCallback)
+                .function(arrangement.logoutCallback::invoke)
+                .with(any<UserId>(), eq(reason))
+                .wasInvoked()
         }
     }
 
@@ -209,6 +210,9 @@ class LogoutUseCaseTest {
         logoutUseCase.invoke(reason)
         arrangement.globalTestScope.advanceUntilIdle()
 
+        verify(arrangement.deregisterTokenUseCase)
+            .suspendFunction(arrangement.deregisterTokenUseCase::invoke)
+            .wasNotInvoked()
         verify(arrangement.logoutRepository)
             .suspendFunction(arrangement.logoutRepository::logout)
             .wasNotInvoked()
@@ -240,6 +244,9 @@ class LogoutUseCaseTest {
             logoutUseCase.invoke(reason)
             arrangement.globalTestScope.advanceUntilIdle()
 
+            verify(arrangement.deregisterTokenUseCase)
+                .suspendFunction(arrangement.deregisterTokenUseCase::invoke)
+                .wasInvoked(exactly = once)
             verify(arrangement.logoutRepository)
                 .suspendFunction(arrangement.logoutRepository::logout)
                 .wasInvoked(exactly = once)
@@ -278,6 +285,9 @@ class LogoutUseCaseTest {
         logoutUseCase.invoke(reason)
         arrangement.globalTestScope.advanceUntilIdle()
 
+        verify(arrangement.deregisterTokenUseCase)
+            .suspendFunction(arrangement.deregisterTokenUseCase::invoke)
+            .wasInvoked(exactly = once)
         verify(arrangement.logoutRepository)
             .suspendFunction(arrangement.logoutRepository::logout)
             .wasInvoked(exactly = once)
@@ -318,6 +328,9 @@ class LogoutUseCaseTest {
         logoutUseCase.invoke(reason)
         arrangement.globalTestScope.advanceUntilIdle()
 
+        verify(arrangement.deregisterTokenUseCase)
+            .suspendFunction(arrangement.deregisterTokenUseCase::invoke)
+            .wasInvoked(exactly = once)
         verify(arrangement.logoutRepository)
             .suspendFunction(arrangement.logoutRepository::logout)
             .wasInvoked(exactly = once)
@@ -365,6 +378,9 @@ class LogoutUseCaseTest {
         @Mock
         val endCall = configure(mock(EndCallUseCase::class)) { stubsUnitByDefault = true }
 
+        @Mock
+        val logoutCallback = configure(mock(classOf<LogoutCallback>())) { stubsUnitByDefault = true }
+
         var kaliumConfigs = KaliumConfigs()
 
         val globalTestScope = TestScope()
@@ -384,6 +400,7 @@ class LogoutUseCaseTest {
                 userSessionWorkScheduler,
                 observeEstablishedCallsUseCase,
                 endCall,
+                logoutCallback,
                 kaliumConfigs
             )
-Original file line number
+Diff line change
@@ Expand Up / @@ -44,7 +44,7 @@ internal interface SyncCriteriaResolution { @@
         /**
          * At least a criterion is not satisfied
          */
-        class MissingRequirement(val cause: String) : SyncCriteriaResolution
+        data class MissingRequirement(val cause: String) : SyncCriteriaResolution
     }
     /**
@@ Expand Down @@