Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: missing ServerConfig crashes after session expired / logout [WPB-5960] #2354

Merged
merged 2 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ actual class CoreLogic(
globalCallManager,
userStorageProvider,
networkStateObserver,
logoutCallbackManager,
userAgent
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -52,6 +53,7 @@ internal fun UserSessionScope(
userStorageProvider: UserStorageProvider,
userSessionScopeProvider: UserSessionScopeProvider,
networkStateObserver: NetworkStateObserver,
logoutCallback: LogoutCallback,
): UserSessionScope {
val platformUserStorageProperties =
PlatformUserStorageProperties(applicationContext, SecurityHelperImpl(globalPreferences.passphraseStorage))
Expand All @@ -73,6 +75,7 @@ internal fun UserSessionScope(
userStorageProvider,
clientConfig,
platformUserStorageProperties,
networkStateObserver
networkStateObserver,
logoutCallback
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {

Expand All @@ -73,6 +75,7 @@ internal actual class UserSessionScopeProviderImpl(
userStorageProvider = userStorageProvider,
userSessionScopeProvider = this,
networkStateObserver = networkStateObserver,
logoutCallback = logoutCallback,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ actual class CoreLogic(
globalCallManager,
userStorageProvider,
networkStateObserver,
logoutCallbackManager,
userAgent
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -49,6 +50,7 @@ internal fun UserSessionScope(
userStorageProvider: UserStorageProvider,
userSessionScopeProvider: UserSessionScopeProvider,
networkStateObserver: NetworkStateObserver,
logoutCallback: LogoutCallback,
userAgent: String
): UserSessionScope {

Expand All @@ -69,6 +71,7 @@ internal fun UserSessionScope(
userStorageProvider,
clientConfig,
platformUserStorageProperties,
networkStateObserver
networkStateObserver,
logoutCallback
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -69,6 +71,7 @@ internal actual class UserSessionScopeProviderImpl(
userStorageProvider,
this,
networkStateObserver,
logoutCallback,
userAgent
)
}
Expand Down
10 changes: 8 additions & 2 deletions logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreLogic.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -1696,6 +1698,7 @@ class UserSessionScope internal constructor(
userSessionWorkScheduler,
calls.establishedCall,
calls.endCall,
logoutCallback,
kaliumConfigs
)
val persistPersistentWebSocketConnectionStatus: PersistPersistentWebSocketConnectionStatusUseCase
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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()
}

Expand Down Expand Up @@ -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 }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand All @@ -40,5 +41,5 @@ class CurrentSessionFlowUseCase(private val sessionRepository: SessionRepository
}, {
CurrentSessionResult.Success(it)
})
}
}.distinctUntilChanged()
}
Original file line number Diff 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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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()
}
}

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -384,6 +400,7 @@ class LogoutUseCaseTest {
userSessionWorkScheduler,
observeEstablishedCallsUseCase,
endCall,
logoutCallback,
kaliumConfigs
)

Expand Down
Loading
Loading