From 16bfd5d771d3d6629cf2eb542a474bcb8f61d705 Mon Sep 17 00:00:00 2001 From: Oussama Hassine Date: Thu, 14 Mar 2024 18:17:37 +0100 Subject: [PATCH] fix: remove CRL check for current client (WPB-7125) (#2655) * fix: remove CRL check for current client * chore: remove CRL check for self client * chore: detekt --- .../configuration/UserConfigRepository.kt | 7 - .../CertificateRevocationListRepository.kt | 22 -- .../kalium/logic/feature/UserSessionScope.kt | 13 -- ...ckRevocationListForCurrentClientUseCase.kt | 60 ----- .../usecase/CheckRevocationListUseCase.kt | 2 - ...vocationListForCurrentClientUseCaseTest.kt | 208 ------------------ .../persistence/dao/unread/UserConfigDAO.kt | 11 - 7 files changed, 323 deletions(-) delete mode 100644 logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListForCurrentClientUseCase.kt delete mode 100644 logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListForCurrentClientUseCaseTest.kt 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 e5244915844..1041832636e 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 @@ -122,8 +122,6 @@ interface UserConfigRepository { suspend fun setLegalHoldChangeNotified(isNotified: Boolean): Either suspend fun observeLegalHoldChangeNotified(): Flow> suspend fun setShouldUpdateClientLegalHoldCapability(shouldUpdate: Boolean): Either - suspend fun shouldCheckCrlForCurrentClient(): Boolean - suspend fun setShouldCheckCrlForCurrentClient(shouldCheck: Boolean): Either suspend fun shouldUpdateClientLegalHoldCapability(): Boolean suspend fun setCRLExpirationTime(url: String, timestamp: ULong) suspend fun getCRLExpirationTime(url: String): ULong? @@ -451,11 +449,6 @@ internal class UserConfigDataSource internal constructor( override suspend fun shouldUpdateClientLegalHoldCapability(): Boolean = userConfigDAO.shouldUpdateClientLegalHoldCapability() - override suspend fun shouldCheckCrlForCurrentClient() = userConfigDAO.shouldCheckCrlForCurrentClient() - - override suspend fun setShouldCheckCrlForCurrentClient(shouldCheck: Boolean): Either = - wrapStorageRequest { userConfigDAO.setShouldCheckCrlForCurrentClient(shouldCheck) } - override suspend fun setCRLExpirationTime(url: String, timestamp: ULong) { userConfigDAO.setCRLExpirationTime(url, timestamp) } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/e2ei/CertificateRevocationListRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/e2ei/CertificateRevocationListRepository.kt index 18867e69f33..126a095d1eb 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/e2ei/CertificateRevocationListRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/e2ei/CertificateRevocationListRepository.kt @@ -18,19 +18,13 @@ package com.wire.kalium.logic.data.e2ei import com.wire.kalium.logic.CoreFailure -import com.wire.kalium.logic.E2EIFailure import com.wire.kalium.logic.configuration.UserConfigRepository import com.wire.kalium.logic.functional.Either -import com.wire.kalium.logic.functional.flatMap -import com.wire.kalium.logic.functional.left -import com.wire.kalium.logic.functional.right import com.wire.kalium.logic.wrapApiRequest import com.wire.kalium.network.api.base.unbound.acme.ACMEApi import com.wire.kalium.persistence.config.CRLUrlExpirationList import com.wire.kalium.persistence.config.CRLWithExpiration import com.wire.kalium.persistence.dao.MetadataDAO -import io.ktor.http.URLBuilder -import io.ktor.http.authority interface CertificateRevocationListRepository { @@ -41,7 +35,6 @@ interface CertificateRevocationListRepository { */ suspend fun getCRLs(): CRLUrlExpirationList? suspend fun addOrUpdateCRL(url: String, timestamp: ULong) - suspend fun getCurrentClientCrlUrl(): Either suspend fun getClientDomainCRL(url: String): Either } @@ -85,20 +78,6 @@ internal class CertificateRevocationListRepositoryDataSource( ) } - override suspend fun getCurrentClientCrlUrl(): Either = - userConfigRepository.getE2EISettings() - .flatMap { - if (!it.isRequired) E2EIFailure.Disabled.left() - else if (it.discoverUrl == null) E2EIFailure.MissingDiscoveryUrl.left() - else URLBuilder(it.discoverUrl).apply { - pathSegments.lastOrNull().let { segment -> - if (segment == null || segment != PATH_CRL) { - pathSegments = pathSegments + PATH_CRL - } - } - }.authority.right() - } - override suspend fun getClientDomainCRL(url: String): Either = wrapApiRequest { acmeApi.getClientDomainCRL(url) @@ -106,6 +85,5 @@ internal class CertificateRevocationListRepositoryDataSource( companion object { const val CRL_LIST_KEY = "crl_list_key" - const val PATH_CRL = "crl" } } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt index 6d945b5db5a..f8b5ae82b20 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt @@ -211,8 +211,6 @@ import com.wire.kalium.logic.feature.e2ei.ACMECertificatesSyncWorker import com.wire.kalium.logic.feature.e2ei.ACMECertificatesSyncWorkerImpl import com.wire.kalium.logic.feature.e2ei.CertificateRevocationListCheckWorker import com.wire.kalium.logic.feature.e2ei.CertificateRevocationListCheckWorkerImpl -import com.wire.kalium.logic.feature.e2ei.usecase.CheckRevocationListForCurrentClientUseCase -import com.wire.kalium.logic.feature.e2ei.usecase.CheckRevocationListForCurrentClientUseCaseImpl import com.wire.kalium.logic.feature.e2ei.usecase.CheckRevocationListUseCase import com.wire.kalium.logic.feature.e2ei.usecase.CheckRevocationListUseCaseImpl import com.wire.kalium.logic.feature.featureConfig.FeatureFlagSyncWorkerImpl @@ -646,13 +644,6 @@ class UserSessionScope internal constructor( mlsClientProvider = mlsClientProvider, isE2EIEnabledUseCase = isE2EIEnabled ) - private val checkRevocationListForCurrentClient: CheckRevocationListForCurrentClientUseCase - get() = CheckRevocationListForCurrentClientUseCaseImpl( - checkRevocationList = checkRevocationList, - certificateRevocationListRepository = certificateRevocationListRepository, - userConfigRepository = userConfigRepository, - isE2EIEnabledUseCase = isE2EIEnabled - ) private val mlsConversationRepository: MLSConversationRepository get() = MLSConversationDataSource( @@ -2033,10 +2024,6 @@ class UserSessionScope internal constructor( certificateRevocationListCheckWorker.execute() } - launch { - checkRevocationListForCurrentClient.invoke() - } - launch { avsSyncStateReporter.execute() } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListForCurrentClientUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListForCurrentClientUseCase.kt deleted file mode 100644 index e027101f689..00000000000 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListForCurrentClientUseCase.kt +++ /dev/null @@ -1,60 +0,0 @@ -/* - * 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.e2ei.usecase - -import com.wire.kalium.logic.configuration.UserConfigRepository -import com.wire.kalium.logic.data.e2ei.CertificateRevocationListRepository -import com.wire.kalium.logic.feature.e2ei.CertificateRevocationListCheckWorker -import com.wire.kalium.logic.feature.user.IsE2EIEnabledUseCase -import com.wire.kalium.logic.functional.onFailure -import com.wire.kalium.logic.functional.onSuccess -import com.wire.kalium.logic.kaliumLogger - -/** - * Use case to check the revocation list for the current client and store it in the DB. - * This will run once as [CertificateRevocationListCheckWorker] is constantly checking the CRLs that are stored in DB. - */ -interface CheckRevocationListForCurrentClientUseCase { - suspend operator fun invoke() -} - -internal class CheckRevocationListForCurrentClientUseCaseImpl( - private val checkRevocationList: CheckRevocationListUseCase, - private val certificateRevocationListRepository: CertificateRevocationListRepository, - private val userConfigRepository: UserConfigRepository, - private val isE2EIEnabledUseCase: IsE2EIEnabledUseCase -) : CheckRevocationListForCurrentClientUseCase { - override suspend fun invoke() { - if (isE2EIEnabledUseCase() && userConfigRepository.shouldCheckCrlForCurrentClient()) { - certificateRevocationListRepository.getCurrentClientCrlUrl().onSuccess { url -> - kaliumLogger.i("Checking CRL for current client..") - checkRevocationList(url) - .onSuccess { expiration -> - kaliumLogger.i("Successfully checked CRL for current client..") - expiration?.let { - certificateRevocationListRepository.addOrUpdateCRL(url, it) - userConfigRepository.setShouldCheckCrlForCurrentClient(false) - } - } - .onFailure { - kaliumLogger.i("Failed to check CRL for current client..") - } - } - } - } -} diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListUseCase.kt index 9e37978e679..4ce4d887eae 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListUseCase.kt @@ -44,9 +44,7 @@ internal class CheckRevocationListUseCaseImpl( private val logger = kaliumLogger.withTextTag("CheckRevocationListUseCase") override suspend fun invoke(url: String): Either { return if (isE2EIEnabledUseCase()) { - logger.i("checking crl url: $url") - certificateRevocationListRepository.getClientDomainCRL(url).flatMap { currentClientIdProvider().flatMap { clientId -> mlsClientProvider.getCoreCrypto(clientId).map { coreCrypto -> diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListForCurrentClientUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListForCurrentClientUseCaseTest.kt deleted file mode 100644 index 8cd605c0b04..00000000000 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListForCurrentClientUseCaseTest.kt +++ /dev/null @@ -1,208 +0,0 @@ -/* - * 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.e2ei.usecase - -import com.wire.kalium.logic.CoreFailure -import com.wire.kalium.logic.configuration.UserConfigRepository -import com.wire.kalium.logic.data.e2ei.CertificateRevocationListRepository -import com.wire.kalium.logic.feature.user.IsE2EIEnabledUseCase -import com.wire.kalium.logic.functional.Either -import io.mockative.Mock -import io.mockative.any -import io.mockative.classOf -import io.mockative.eq -import io.mockative.given -import io.mockative.mock -import io.mockative.once -import io.mockative.verify -import kotlinx.coroutines.test.runTest -import kotlin.test.Test - -class CheckRevocationListForCurrentClientUseCaseTest { - - @Test - fun givenE2eiDisabled_whenRunningUseCase_thenDoNothing() = runTest { - val (arrangement, checkRevocationListForCurrentClient) = Arrangement() - .withE2eiDisabled() - .arrange() - - checkRevocationListForCurrentClient() - - verify(arrangement.certificateRevocationListRepository) - .suspendFunction(arrangement.certificateRevocationListRepository::getCurrentClientCrlUrl) - .wasNotInvoked() - } - - @Test - fun givenShouldCheckCrlIsFalse_whenRunningUseCase_thenDoNothing() = runTest { - val (arrangement, checkRevocationListForCurrentClient) = Arrangement() - .withShouldCheckCrlForCurrentClientReturning(false) - .withE2eiEnabled() - .arrange() - - checkRevocationListForCurrentClient() - - verify(arrangement.certificateRevocationListRepository) - .suspendFunction(arrangement.certificateRevocationListRepository::getCurrentClientCrlUrl) - .wasNotInvoked() - } - - @Test - fun givenFailureWhenGettingCurrentClientCrlUrl_whenRunningUseCase_thenDoNotCheckCrl() = - runTest { - val (arrangement, checkRevocationListForCurrentClient) = Arrangement() - .withShouldCheckCrlForCurrentClientReturning(true) - .withE2eiEnabled() - .withGetCurrentClientCrlUrlReturning(Either.Left(CoreFailure.InvalidEventSenderID)) - .arrange() - - checkRevocationListForCurrentClient() - - verify(arrangement.certificateRevocationListRepository) - .suspendFunction(arrangement.certificateRevocationListRepository::getCurrentClientCrlUrl) - .wasInvoked(once) - - verify(arrangement.checkRevocationList) - .suspendFunction(arrangement.checkRevocationList::invoke) - .with(any()) - .wasNotInvoked() - } - - @Test - fun givenCheckRevocationListUseCaseReturnsFailure_whenRunningUseCase_thenDoNothing() = runTest { - val (arrangement, checkRevocationListForCurrentClient) = Arrangement() - .withShouldCheckCrlForCurrentClientReturning(true) - .withE2eiEnabled() - .withGetCurrentClientCrlUrlReturning(Either.Right(URL)) - .withCheckRevocationListUseCaseReturning(Either.Left(CoreFailure.InvalidEventSenderID)) - .arrange() - - checkRevocationListForCurrentClient() - - verify(arrangement.checkRevocationList) - .suspendFunction(arrangement.checkRevocationList::invoke) - .with(any()) - .wasInvoked(once) - - verify(arrangement.certificateRevocationListRepository) - .suspendFunction(arrangement.certificateRevocationListRepository::addOrUpdateCRL) - .with(any(), any()) - .wasNotInvoked() - } - - @Test - fun givenCheckRevocationListUseCaseReturnsSuccess_whenRunningUseCase_thenInvokeAddOrUpdateCRLAndSetShouldCheckCrlForCurrentClientOnce() = - runTest { - val (arrangement, checkRevocationListForCurrentClient) = Arrangement() - .withShouldCheckCrlForCurrentClientReturning(true) - .withGetCurrentClientCrlUrlReturning(Either.Right(URL)) - .withE2eiEnabled() - .withCheckRevocationListUseCaseReturning(Either.Right(EXPIRATION)) - .withSetShouldCheckCrlForCurrentClient() - .arrange() - - checkRevocationListForCurrentClient() - - verify(arrangement.checkRevocationList) - .suspendFunction(arrangement.checkRevocationList::invoke) - .with(any()) - .wasInvoked(once) - - verify(arrangement.certificateRevocationListRepository) - .suspendFunction(arrangement.certificateRevocationListRepository::addOrUpdateCRL) - .with(any(), any()) - .wasInvoked(once) - - verify(arrangement.userConfigRepository) - .suspendFunction(arrangement.userConfigRepository::setShouldCheckCrlForCurrentClient) - .with(eq(false)) - .wasInvoked(once) - } - - internal class Arrangement { - - @Mock - val certificateRevocationListRepository = - mock(classOf()) - - @Mock - val checkRevocationList = mock(classOf()) - - @Mock - val userConfigRepository = - mock(classOf()) - - @Mock - val isE2EIEnabled = - mock(classOf()) - - fun arrange() = this to CheckRevocationListForCurrentClientUseCaseImpl( - checkRevocationList = checkRevocationList, - certificateRevocationListRepository = certificateRevocationListRepository, - userConfigRepository = userConfigRepository, - isE2EIEnabledUseCase = isE2EIEnabled - ) - - fun withE2eiEnabled() = apply { - given(isE2EIEnabled) - .function(isE2EIEnabled::invoke) - .whenInvoked() - .thenReturn(true) - } - - fun withE2eiDisabled() = apply { - given(isE2EIEnabled) - .function(isE2EIEnabled::invoke) - .whenInvoked() - .thenReturn(false) - } - - fun withShouldCheckCrlForCurrentClientReturning(value: Boolean) = apply { - given(userConfigRepository) - .suspendFunction(userConfigRepository::shouldCheckCrlForCurrentClient) - .whenInvoked() - .thenReturn(value) - } - - fun withGetCurrentClientCrlUrlReturning(result: Either) = apply { - given(certificateRevocationListRepository) - .suspendFunction(certificateRevocationListRepository::getCurrentClientCrlUrl) - .whenInvoked() - .thenReturn(result) - } - - fun withCheckRevocationListUseCaseReturning(result: Either) = apply { - given(checkRevocationList) - .suspendFunction(checkRevocationList::invoke) - .whenInvokedWith(any()) - .thenReturn(result) - } - - fun withSetShouldCheckCrlForCurrentClient() = apply { - given(userConfigRepository) - .suspendFunction(userConfigRepository::setShouldCheckCrlForCurrentClient) - .whenInvokedWith(any()) - .thenReturn(Either.Right(Unit)) - } - } - - companion object { - private const val URL = "https://example.com" - private const val EXPIRATION = 12324UL - } -} \ No newline at end of file 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 a1ebf0fe7e4..0b9671ba6bc 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 @@ -52,8 +52,6 @@ interface UserConfigDAO { suspend fun observeLegalHoldChangeNotified(): Flow suspend fun setShouldUpdateClientLegalHoldCapability(shouldUpdate: Boolean) suspend fun shouldUpdateClientLegalHoldCapability(): Boolean - suspend fun setShouldCheckCrlForCurrentClient(shouldCheck: Boolean) - suspend fun shouldCheckCrlForCurrentClient(): Boolean suspend fun setCRLExpirationTime(url: String, timestamp: ULong) suspend fun getCRLsPerDomain(url: String): ULong? suspend fun observeCertificateExpirationTime(url: String): Flow @@ -158,13 +156,6 @@ internal class UserConfigDAOImpl internal constructor( override suspend fun shouldUpdateClientLegalHoldCapability(): Boolean = metadataDAO.valueByKey(SHOULD_UPDATE_CLIENT_LEGAL_HOLD_CAPABILITY)?.toBoolean() ?: true - override suspend fun setShouldCheckCrlForCurrentClient(shouldCheck: Boolean) { - metadataDAO.insertValue(shouldCheck.toString(), SHOULD_CHECK_CRL_CURRENT_CLIENT) - } - - override suspend fun shouldCheckCrlForCurrentClient(): Boolean = - metadataDAO.valueByKey(SHOULD_CHECK_CRL_CURRENT_CLIENT)?.toBoolean() ?: true - override suspend fun setCRLExpirationTime(url: String, timestamp: ULong) { metadataDAO.insertValue( key = url, @@ -194,7 +185,5 @@ 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_CHECK_CRL_CURRENT_CLIENT = - "should_check_crl_current_client" } }