fix: crash when login after session expire and client deleted remotel…

…y [WPB-11061] (#3031) * revert me:fix login after deleting client * fix: crash when login after deleting client and session expires * cleanup * detekt * tests * tests
wireapp · Sep 25, 2024 · bea029d · bea029d
1 parent c0869d4
commit bea029d
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 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
@@ -136,8 +136,8 @@ interface UserConfigRepository {
     suspend fun setShouldNotifyForRevokedCertificate(shouldNotify: Boolean)
     suspend fun observeShouldNotifyForRevokedCertificate(): Flow<Either<StorageFailure, Boolean>>
     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<Either<StorageFailure, Boolean>> =
         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
@@ -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<CoreFailure, MLSClient> {
+    private suspend fun mlsClient(
+        userId: CryptoUserID,
+        clientId: ClientId
+    ): Either<CoreFailure, MLSClient> {
         return getCoreCrypto(clientId).flatMap { cc ->
             getOrFetchMLSConfig().map { (supportedCipherSuite, defaultCipherSuite) ->
                 cc.mlsClient(

diff --git a/.../src/commonMain/kotlin/com/wire/kalium/logic/feature/client/GetOrRegisterClientUseCase.kt b/.../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
@@ -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
@@ -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
@@ -178,8 +178,6 @@ interface UserConfigStorage {
     fun getE2EINotificationTime(): Long?
     fun e2EINotificationTimeFlow(): Flow<Long?>
     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
@@ -60,6 +60,8 @@ interface UserConfigDAO {
     suspend fun observeShouldNotifyForRevokedCertificate(): Flow<Boolean?>
     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
@@ -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())
+    }
 }