From 752c1248b25815bcc6302439d942dca7aaa2ca3d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 26 Sep 2024 12:33:31 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20crash=20when=20login=20after=20session?= =?UTF-8?q?=20expire=20and=20client=20deleted=20remotely=20[WPB-11061]=20?= =?UTF-8?q?=F0=9F=8D=92=20=F0=9F=8D=92=20(#3040)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Commit with unresolved merge conflicts * fix merge conflicts --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Mohamad Jaara --- .../logic/configuration/UserConfigRepository.kt | 11 +++++------ .../logic/data/client/MLSClientProvider.kt | 14 ++++++++++---- .../client/GetOrRegisterClientUseCase.kt | 2 +- .../feature/client/RegisterClientUseCase.kt | 2 +- .../logic/data/e2ei/E2EIRepositoryTest.kt | 13 +++++++------ .../persistence/config/UserConfigStorage.kt | 10 ---------- .../persistence/dao/unread/UserConfigDAO.kt | 10 ++++++++++ .../persistence/config/UserConfigDAOTest.kt | 17 +++++++++++++++++ 8 files changed, 51 insertions(+), 28 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 b49ac5926b6..71a15a7f63f 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,6 @@ interface UserConfigRepository { suspend fun setShouldNotifyForRevokedCertificate(shouldNotify: Boolean) suspend fun observeShouldNotifyForRevokedCertificate(): Flow> suspend fun clearE2EISettings() - fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) - fun getShouldFetchE2EITrustAnchor(): Boolean suspend fun setCurrentTrackingIdentifier(newIdentifier: String) suspend fun getCurrentTrackingIdentifier(): String? suspend fun observeCurrentTrackingIdentifier(): Flow> @@ -146,6 +144,8 @@ interface UserConfigRepository { suspend fun deletePreviousTrackingIdentifier() suspend fun updateNextTimeForCallFeedback(valueMs: Long) suspend fun getNextTimeForCallFeedback(): Either + suspend fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) + suspend fun getShouldFetchE2EITrustAnchor(): Boolean } @Suppress("TooManyFunctions") @@ -508,12 +508,10 @@ 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 setCurrentTrackingIdentifier(newIdentifier: String) { wrapStorageRequest { userConfigDAO.setTrackingIdentifier(identifier = newIdentifier) @@ -547,4 +545,5 @@ internal class UserConfigDataSource internal constructor( override suspend fun getNextTimeForCallFeedback() = wrapStorageRequest { userConfigDAO.getNextTimeForCallFeedback() } + 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 18369dcda3d..97d2dc0afe8 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 0b544f029f3..bd53f5a1e33 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 @@ -55,7 +55,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 c2d602d9c12..9b2a097ddc8 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 @@ -144,7 +144,7 @@ class RegisterClientUseCaseImpl @OptIn(DelicateKaliumApi::class) internal constr model, cookieLabel, verificationCode, - modelPostfix + modelPostfix, ) }.fold({ RegisterClientResult.Failure.Generic(it) 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 7e7d7f0edf2..8601cd4988b 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 @@ -914,7 +914,7 @@ class E2EIRepositoryTest { arrangement.coreCryptoCentral.registerTrustAnchors(eq(Arrangement.RANDOM_BYTE_ARRAY.decodeToString())) }.wasInvoked(once) - verify { + coVerify { arrangement.userConfigRepository.setShouldFetchE2EITrustAnchors(eq(false)) }.wasInvoked(once) } @@ -933,7 +933,7 @@ class E2EIRepositoryTest { // then result.shouldSucceed() - verify { + coVerify { arrangement.userConfigRepository.getShouldFetchE2EITrustAnchor() }.wasInvoked(once) } @@ -1099,18 +1099,19 @@ class E2EIRepositoryTest { }.returns(result) } - fun withGetShouldFetchE2EITrustAnchors(result: Boolean) = apply { - every { + suspend fun withGetShouldFetchE2EITrustAnchors(result: Boolean) = apply { + coEvery { userConfigRepository.getShouldFetchE2EITrustAnchor() }.returns(result) } - fun withSetShouldFetchE2EIGetTrustAnchors() = apply { - every { + suspend fun withSetShouldFetchE2EIGetTrustAnchors() = apply { + coEvery { userConfigRepository.setShouldFetchE2EITrustAnchors(any()) }.returns(Unit) } + suspend fun withAcmeDirectoriesApiSucceed() = apply { coEvery { acmeApi.getACMEDirectories(any()) 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 ab36fd9b728..6cbf40001ac 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 @@ -68,6 +68,8 @@ interface UserConfigDAO { suspend fun deletePreviousTrackingIdentifier() suspend fun getNextTimeForCallFeedback(): Long? suspend fun setNextTimeForCallFeedback(timestamp: Long) + suspend fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean) + suspend fun getShouldFetchE2EITrustAnchorHasRun(): Boolean } @Suppress("TooManyFunctions") @@ -226,6 +228,13 @@ internal class UserConfigDAOImpl internal constructor( override suspend fun setNextTimeForCallFeedback(timestamp: Long) = metadataDAO.insertValue(timestamp.toString(), NEXT_TIME_TO_ASK_CALL_FEEDBACK) + 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" @@ -239,5 +248,6 @@ internal class UserConfigDAOImpl internal constructor( "should_update_client_legal_hold_capability" private const val ANALYTICS_TRACKING_IDENTIFIER_PREVIOUS_KEY = "analytics_tracking_identifier_previous" private const val ANALYTICS_TRACKING_IDENTIFIER_KEY = "analytics_tracking_identifier" + 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()) + } }