Skip to content

Commit

Permalink
fix: crash when login after session expire and client deleted remotel…
Browse files Browse the repository at this point in the history
…y [WPB-11061] 🍒 🍒 (#3040)

* 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 <[email protected]>
  • Loading branch information
github-actions[bot] and MohamadJaara authored Sep 26, 2024
1 parent f2dc281 commit 752c124
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,6 @@ 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 setCurrentTrackingIdentifier(newIdentifier: String)
suspend fun getCurrentTrackingIdentifier(): String?
suspend fun observeCurrentTrackingIdentifier(): Flow<Either<StorageFailure, String>>
Expand All @@ -146,6 +144,8 @@ interface UserConfigRepository {
suspend fun deletePreviousTrackingIdentifier()
suspend fun updateNextTimeForCallFeedback(valueMs: Long)
suspend fun getNextTimeForCallFeedback(): Either<StorageFailure, Long>
suspend fun setShouldFetchE2EITrustAnchors(shouldFetch: Boolean)
suspend fun getShouldFetchE2EITrustAnchor(): Boolean
}

@Suppress("TooManyFunctions")
Expand Down Expand Up @@ -508,12 +508,10 @@ 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 setCurrentTrackingIdentifier(newIdentifier: String) {
wrapStorageRequest {
userConfigDAO.setTrackingIdentifier(identifier = newIdentifier)
Expand Down Expand Up @@ -547,4 +545,5 @@ internal class UserConfigDataSource internal constructor(

override suspend fun getNextTimeForCallFeedback() = wrapStorageRequest { userConfigDAO.getNextTimeForCallFeedback() }

override suspend fun getShouldFetchE2EITrustAnchor(): Boolean = userConfigDAO.getShouldFetchE2EITrustAnchorHasRun()
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class RegisterClientUseCaseImpl @OptIn(DelicateKaliumApi::class) internal constr
model,
cookieLabel,
verificationCode,
modelPostfix
modelPostfix,
)
}.fold({
RegisterClientResult.Failure.Generic(it)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -933,7 +933,7 @@ class E2EIRepositoryTest {
// then
result.shouldSucceed()

verify {
coVerify {
arrangement.userConfigRepository.getShouldFetchE2EITrustAnchor()
}.wasInvoked(once)
}
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand All @@ -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"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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"
Expand All @@ -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"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}

0 comments on commit 752c124

Please sign in to comment.