From bea029d86e57ae0d46d9923595d3ad467c5b8c57 Mon Sep 17 00:00:00 2001 From: Mohamad Jaara Date: Wed, 25 Sep 2024 12:21:14 +0200 Subject: [PATCH] fix: crash when login after session expire and client deleted remotely [WPB-11061] (#3031) * revert me:fix login after deleting client * fix: crash when login after deleting client and session expires * cleanup * detekt * tests * tests --- .../logic/configuration/UserConfigRepository.kt | 10 +++++----- .../logic/data/client/MLSClientProvider.kt | 14 ++++++++++---- .../client/GetOrRegisterClientUseCase.kt | 2 +- .../feature/client/RegisterClientUseCase.kt | 10 +++++++++- .../logic/data/e2ei/E2EIRepositoryTest.kt | 6 +++--- .../persistence/config/UserConfigStorage.kt | 10 ---------- .../persistence/dao/unread/UserConfigDAO.kt | 10 ++++++++++ .../persistence/config/UserConfigDAOTest.kt | 17 +++++++++++++++++ 8 files changed, 55 insertions(+), 24 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/UserConfigRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/UserConfigRepository.kt index 29424e92db5..e82f799b4b6 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/UserConfigRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/UserConfigRepository.kt @@ -136,8 +136,8 @@ interface UserConfigRepository { suspend fun setShouldNotifyForRevokedCertificate(shouldNotify: Boolean) suspend fun observeShouldNotifyForRevokedCertificate(): Flow> suspend fun clearE2EISettings() - fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) - fun getShouldFetchE2EITrustAnchor(): Boolean + suspend fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) + suspend fun getShouldFetchE2EITrustAnchor(): Boolean } @Suppress("TooManyFunctions") @@ -500,9 +500,9 @@ internal class UserConfigDataSource internal constructor( override suspend fun observeShouldNotifyForRevokedCertificate(): Flow> = userConfigDAO.observeShouldNotifyForRevokedCertificate().wrapStorageRequest() - override fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) { - userConfigStorage.setShouldFetchE2EITrustAnchors(shouldFetch = shouldFetch) + override suspend fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) { + userConfigDAO.setShouldFetchE2EITrustAnchors(shouldFetch = shouldFetch) } - override fun getShouldFetchE2EITrustAnchor(): Boolean = userConfigStorage.getShouldFetchE2EITrustAnchorHasRun() + override suspend fun getShouldFetchE2EITrustAnchor(): Boolean = userConfigDAO.getShouldFetchE2EITrustAnchorHasRun() } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/client/MLSClientProvider.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/client/MLSClientProvider.kt index c5afd316cf8..ce242232fb1 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/client/MLSClientProvider.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/client/MLSClientProvider.kt @@ -141,9 +141,12 @@ class MLSClientProviderImpl( override suspend fun clearLocalFiles() { mlsClientMutex.withLock { - mlsClient?.close() - mlsClient = null - FileUtil.deleteDirectory(rootKeyStorePath) + coreCryptoCentralMutex.withLock { + mlsClient?.close() + mlsClient = null + coreCryptoCentral = null + FileUtil.deleteDirectory(rootKeyStorePath) + } } } @@ -182,7 +185,10 @@ class MLSClientProviderImpl( } } - private suspend fun mlsClient(userId: CryptoUserID, clientId: ClientId): Either { + private suspend fun mlsClient( + userId: CryptoUserID, + clientId: ClientId + ): Either { return getCoreCrypto(clientId).flatMap { cc -> getOrFetchMLSConfig().map { (supportedCipherSuite, defaultCipherSuite) -> cc.mlsClient( diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/GetOrRegisterClientUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/GetOrRegisterClientUseCase.kt index 8d4d55d2ada..85173b1e8cd 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/GetOrRegisterClientUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/GetOrRegisterClientUseCase.kt @@ -53,7 +53,7 @@ internal class GetOrRegisterClientUseCaseImpl( ) : GetOrRegisterClientUseCase { override suspend fun invoke(registerClientParam: RegisterClientUseCase.RegisterClientParam): RegisterClientResult { - syncFeatureConfigsUseCase.invoke() + syncFeatureConfigsUseCase() val result: RegisterClientResult = clientRepository.retainedClientId() .nullableFold( diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/RegisterClientUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/RegisterClientUseCase.kt index 371ff3458bd..0a123c63388 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/RegisterClientUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/client/RegisterClientUseCase.kt @@ -135,7 +135,15 @@ class RegisterClientUseCaseImpl @OptIn(DelicateKaliumApi::class) internal constr val verificationCode = registerClientParam.secondFactorVerificationCode ?: currentlyStoredVerificationCode() sessionRepository.cookieLabel(selfUserId) .flatMap { cookieLabel -> - generateProteusPreKeys(preKeysToSend, password, capabilities, clientType, model, cookieLabel, verificationCode) + generateProteusPreKeys( + preKeysToSend, + password, + capabilities, + clientType, + model, + cookieLabel, + verificationCode + ) }.fold({ RegisterClientResult.Failure.Generic(it) }, { registerClientParam -> diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/e2ei/E2EIRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/e2ei/E2EIRepositoryTest.kt index 26f90778c6c..ad75de19ac1 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/e2ei/E2EIRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/e2ei/E2EIRepositoryTest.kt @@ -1017,7 +1017,7 @@ class E2EIRepositoryTest { result.shouldSucceed() verify(arrangement.userConfigRepository) - .function(arrangement.userConfigRepository::getShouldFetchE2EITrustAnchor) + .suspendFunction(arrangement.userConfigRepository::getShouldFetchE2EITrustAnchor) .wasInvoked(once) } @@ -1203,14 +1203,14 @@ class E2EIRepositoryTest { fun withGetShouldFetchE2EITrustAnchors(result: Boolean) = apply { given(userConfigRepository) - .function(userConfigRepository::getShouldFetchE2EITrustAnchor) + .suspendFunction(userConfigRepository::getShouldFetchE2EITrustAnchor) .whenInvoked() .thenReturn(result) } fun withSetShouldFetchE2EIGetTrustAnchors() = apply { given(userConfigRepository) - .function(userConfigRepository::setShouldFetchE2EITrustAnchors) + .suspendFunction(userConfigRepository::setShouldFetchE2EITrustAnchors) .whenInvokedWith(any()) .thenReturn(Unit) } diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/config/UserConfigStorage.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/config/UserConfigStorage.kt index b3542fb10f7..9f24823294d 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/config/UserConfigStorage.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/config/UserConfigStorage.kt @@ -178,8 +178,6 @@ interface UserConfigStorage { fun getE2EINotificationTime(): Long? fun e2EINotificationTimeFlow(): Flow fun updateE2EINotificationTime(timeStamp: Long) - fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) - fun getShouldFetchE2EITrustAnchorHasRun(): Boolean } @Serializable @@ -576,13 +574,6 @@ class UserConfigStorageImpl( } } - override fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) { - kaliumPreferences.putBoolean(SHOULD_FETCH_E2EI_GET_TRUST_ANCHORS, shouldFetch) - } - - override fun getShouldFetchE2EITrustAnchorHasRun(): Boolean = - kaliumPreferences.getBoolean(SHOULD_FETCH_E2EI_GET_TRUST_ANCHORS, true) - private companion object { const val FILE_SHARING = "file_sharing" const val GUEST_ROOM_LINK = "guest_room_link" @@ -601,6 +592,5 @@ class UserConfigStorageImpl( const val ENABLE_TYPING_INDICATOR = "enable_typing_indicator" const val APP_LOCK = "app_lock" const val DEFAULT_PROTOCOL = "default_protocol" - const val SHOULD_FETCH_E2EI_GET_TRUST_ANCHORS = "should_fetch_e2ei_trust_anchors" } } diff --git a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/unread/UserConfigDAO.kt b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/unread/UserConfigDAO.kt index ede3482c411..e5d50626133 100644 --- a/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/unread/UserConfigDAO.kt +++ b/persistence/src/commonMain/kotlin/com/wire/kalium/persistence/dao/unread/UserConfigDAO.kt @@ -60,6 +60,8 @@ interface UserConfigDAO { suspend fun observeShouldNotifyForRevokedCertificate(): Flow suspend fun setDefaultCipherSuite(cipherSuite: SupportedCipherSuiteEntity) suspend fun getDefaultCipherSuite(): SupportedCipherSuiteEntity? + suspend fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) + suspend fun getShouldFetchE2EITrustAnchorHasRun(): Boolean } @Suppress("TooManyFunctions") @@ -186,6 +188,13 @@ internal class UserConfigDAOImpl internal constructor( override suspend fun getDefaultCipherSuite(): SupportedCipherSuiteEntity? = metadataDAO.getSerializable(DEFAULT_CIPHER_SUITE_KEY, SupportedCipherSuiteEntity.serializer()) + override suspend fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) { + metadataDAO.insertValue(value = shouldFetch.toString(), key = SHOULD_FETCH_E2EI_GET_TRUST_ANCHORS) + } + + override suspend fun getShouldFetchE2EITrustAnchorHasRun(): Boolean = + metadataDAO.valueByKey(SHOULD_FETCH_E2EI_GET_TRUST_ANCHORS)?.toBoolean() ?: true + private companion object { private const val DEFAULT_CIPHER_SUITE_KEY = "DEFAULT_CIPHER_SUITE" private const val SELF_DELETING_MESSAGES_KEY = "SELF_DELETING_MESSAGES" @@ -196,5 +205,6 @@ internal class UserConfigDAOImpl internal constructor( const val LEGAL_HOLD_CHANGE_NOTIFIED = "legal_hold_change_notified" const val SHOULD_UPDATE_CLIENT_LEGAL_HOLD_CAPABILITY = "should_update_client_legal_hold_capability" + const val SHOULD_FETCH_E2EI_GET_TRUST_ANCHORS = "should_fetch_e2ei_trust_anchors" } } diff --git a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/config/UserConfigDAOTest.kt b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/config/UserConfigDAOTest.kt index 8a95cc96f37..a1902160ce8 100644 --- a/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/config/UserConfigDAOTest.kt +++ b/persistence/src/commonTest/kotlin/com/wire/kalium/persistence/config/UserConfigDAOTest.kt @@ -123,4 +123,21 @@ class UserConfigDAOTest : BaseDatabaseTest() { assertEquals(thirdExpectedValue, thirdValue) } } + + @Test + fun givenNoValueStoredForShouldFetchE2EITrustAnchorHasRun_whenCalled_thenReturnTrue() = runTest { + assertTrue(userConfigDAO.getShouldFetchE2EITrustAnchorHasRun()) + } + + @Test + fun givenShouldFetchE2EITrustAnchorHasRunIsSetToFalse_whenCalled_thenReturnFalse() = runTest { + userConfigDAO.setShouldFetchE2EITrustAnchors(false) + assertFalse(userConfigDAO.getShouldFetchE2EITrustAnchorHasRun()) + } + + @Test + fun givenShouldFetchE2EITrustAnchorHasRunIsSetToTrue_whenCalled_thenReturnTrue() = runTest { + userConfigDAO.setShouldFetchE2EITrustAnchors(true) + assertTrue(userConfigDAO.getShouldFetchE2EITrustAnchorHasRun()) + } }