fix: e2ei endpoint is called even when the feature is disabled (#2508) (

#2511) * fix: e2ei endpoint is called even when the feature is disabled * detekt * revert unwanted change * revert unwanted change Co-authored-by: Mohamad Jaara <[email protected]> Co-authored-by: Mojtaba Chenani <[email protected]>
wireapp · Feb 21, 2024 · 5e527d3 · 5e527d3
1 parent e41ccac
commit 5e527d3
Show file tree

Hide file tree

Showing 13 changed files with 111 additions and 37 deletions.
diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt
@@ -191,6 +191,9 @@ interface MLSFailure : CoreFailure {
 }
 
 interface E2EIFailure : CoreFailure {
+
+    data object Disabled : E2EIFailure
+    data object MissingDiscoveryUrl : E2EIFailure
     data class FailedInitialization(val step: E2EIEnrollmentResult.E2EIStep) : E2EIFailure
     data class FailedOAuth(val reason: String) : E2EIFailure
     data class FailedFinalization(val step: E2EIEnrollmentResult.E2EIStep) : E2EIFailure

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/E2EISettings.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/configuration/E2EISettings.kt
@@ -22,7 +22,7 @@ import kotlinx.datetime.Instant
 
 data class E2EISettings(
     val isRequired: Boolean,
-    val discoverUrl: String,
+    val discoverUrl: String?,
     val gracePeriodEnd: Instant?
 ) {
 

diff --git a/.../commonMain/kotlin/com/wire/kalium/logic/data/e2ei/CertificateRevocationListRepository.kt b/.../commonMain/kotlin/com/wire/kalium/logic/data/e2ei/CertificateRevocationListRepository.kt
@@ -18,7 +18,6 @@
 package com.wire.kalium.logic.data.e2ei
 
 import com.wire.kalium.logic.CoreFailure
-import com.wire.kalium.logic.configuration.UserConfigRepository
 import com.wire.kalium.logic.functional.Either
 import com.wire.kalium.logic.functional.map
 import com.wire.kalium.logic.wrapApiRequest
@@ -45,7 +44,7 @@ interface CertificateRevocationListRepository {
 internal class CertificateRevocationListRepositoryDataSource(
     private val acmeApi: ACMEApi,
     private val metadataDAO: MetadataDAO,
-    private val userConfigRepository: UserConfigRepository
+    private val e2eiRepository: E2EIRepository
 ) : CertificateRevocationListRepository {
     override suspend fun getCRLs(): CRLUrlExpirationList? =
         metadataDAO.getSerializable(CRL_LIST_KEY, CRLUrlExpirationList.serializer())
@@ -83,8 +82,8 @@ internal class CertificateRevocationListRepositoryDataSource(
     }
 
     override suspend fun getCurrentClientCrlUrl(): Either<CoreFailure, String> =
-        userConfigRepository.getE2EISettings().map {
-            (Url(it.discoverUrl).protocolWithAuthority)
+        e2eiRepository.discoveryUrl().map {
+            Url(it).protocolWithAuthority
         }
 
     override suspend fun getClientDomainCRL(url: String): Either<CoreFailure, ByteArray> =

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/e2ei/E2EIRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/e2ei/E2EIRepository.kt
@@ -24,6 +24,7 @@ import com.wire.kalium.cryptography.AcmeDirectory
 import com.wire.kalium.cryptography.NewAcmeAuthz
 import com.wire.kalium.cryptography.NewAcmeOrder
 import com.wire.kalium.logic.CoreFailure
+import com.wire.kalium.logic.E2EIFailure
 import com.wire.kalium.logic.configuration.UserConfigRepository
 import com.wire.kalium.logic.data.client.E2EIClientProvider
 import com.wire.kalium.logic.data.client.MLSClientProvider
@@ -35,6 +36,7 @@ import com.wire.kalium.logic.functional.Either
 import com.wire.kalium.logic.functional.flatMap
 import com.wire.kalium.logic.functional.fold
 import com.wire.kalium.logic.functional.getOrFail
+import com.wire.kalium.logic.functional.left
 import com.wire.kalium.logic.functional.map
 import com.wire.kalium.logic.kaliumLogger
 import com.wire.kalium.logic.wrapApiRequest
@@ -91,6 +93,7 @@ interface E2EIRepository {
     suspend fun fetchFederationCertificates(): Either<CoreFailure, Unit>
     suspend fun getCurrentClientCrlUrl(): Either<CoreFailure, String>
     suspend fun getClientDomainCRL(url: String): Either<CoreFailure, ByteArray>
+    fun discoveryUrl(): Either<CoreFailure, String>
 }
 
 @Suppress("LongParameterList")
@@ -116,9 +119,9 @@ class E2EIRepositoryImpl(
         })
     }
 
-    override suspend fun fetchAndSetTrustAnchors(): Either<CoreFailure, Unit> = userConfigRepository.getE2EISettings().flatMap {
+    override suspend fun fetchAndSetTrustAnchors(): Either<CoreFailure, Unit> = discoveryUrl().flatMap {
         wrapApiRequest {
-            acmeApi.getTrustAnchors(Url(it.discoverUrl).protocolWithAuthority)
+            acmeApi.getTrustAnchors(Url(it).protocolWithAuthority)
         }.flatMap { trustAnchors ->
             mlsClientProvider.getMLSClient().flatMap { mlsClient ->
                 wrapE2EIRequest {
@@ -128,9 +131,9 @@ class E2EIRepositoryImpl(
         }
     }
 
-    override suspend fun loadACMEDirectories(): Either<CoreFailure, AcmeDirectory> = userConfigRepository.getE2EISettings().flatMap {
+    override suspend fun loadACMEDirectories(): Either<CoreFailure, AcmeDirectory> = discoveryUrl().flatMap {
         wrapApiRequest {
-            acmeApi.getACMEDirectories(it.discoverUrl)
+            acmeApi.getACMEDirectories(it)
         }.flatMap { directories ->
             e2EIClientProvider.getE2EIClient().flatMap { e2eiClient ->
                 wrapE2EIRequest {
@@ -299,26 +302,34 @@ class E2EIRepositoryImpl(
         Either.Right(e2EIClient.getOAuthRefreshToken())
     }
 
-    override suspend fun fetchFederationCertificates(): Either<CoreFailure, Unit> = userConfigRepository.getE2EISettings().flatMap {
-        wrapApiRequest {
-            acmeApi.getACMEFederation(Url(it.discoverUrl).protocolWithAuthority)
-        }.flatMap { data ->
-            mlsClientProvider.getMLSClient().flatMap { mlsClient ->
-                wrapMLSRequest {
-                    mlsClient.registerIntermediateCa(data)
+    override suspend fun fetchFederationCertificates(): Either<CoreFailure, Unit> = discoveryUrl()
+        .flatMap {
+            wrapApiRequest {
+                acmeApi.getACMEFederation(Url(it).protocolWithAuthority)
+            }.flatMap { data ->
+                mlsClientProvider.getMLSClient().flatMap { mlsClient ->
+                    wrapMLSRequest {
+                        mlsClient.registerIntermediateCa(data)
+                    }
                 }
             }
         }
-    }
+
+    override fun discoveryUrl(): Either<CoreFailure, String> =
+        userConfigRepository.getE2EISettings().flatMap { settings ->
+            when {
+                !settings.isRequired -> E2EIFailure.Disabled.left()
+                settings.discoverUrl == null -> E2EIFailure.MissingDiscoveryUrl.left()
+                else -> Either.Right(settings.discoverUrl)
+            }
+        }
 
     override suspend fun nukeE2EIClient() {
         e2EIClientProvider.nuke()
     }
 
     override suspend fun getCurrentClientCrlUrl(): Either<CoreFailure, String> =
-        userConfigRepository.getE2EISettings().map {
-            (Url(it.discoverUrl).protocolWithAuthority)
-        }
+        discoveryUrl().map { Url(it).protocolWithAuthority }
 
     override suspend fun getClientDomainCRL(url: String): Either<CoreFailure, ByteArray> =
         wrapApiRequest {

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/featureConfig/FeatureConfigMapper.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/featureConfig/FeatureConfigMapper.kt
@@ -131,13 +131,22 @@ class FeatureConfigMapperImpl : FeatureConfigMapper {
         )
 
     override fun fromDTO(data: FeatureConfigData.E2EI?): E2EIModel =
-        E2EIModel(
+        data?.let {
+            E2EIModel(
+                E2EIConfigModel(
+                    data.config.url,
+                    data.config.verificationExpirationSeconds
+                ),
+                fromDTO(data.status)
+            )
+        } ?: E2EIModel(
             E2EIConfigModel(
-                data?.config?.url ?: "",
-                data?.config?.verificationExpirationSeconds ?: 0L
+                null,
+                0
             ),
-            fromDTO(data?.status ?: FeatureFlagStatusDTO.DISABLED)
+            Status.DISABLED
         )
+
     override fun fromModel(status: Status): FeatureFlagStatusDTO =
         when (status) {
             Status.ENABLED -> FeatureFlagStatusDTO.ENABLED

diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/featureConfig/FeatureConfigModel.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/featureConfig/FeatureConfigModel.kt
@@ -99,6 +99,6 @@ data class E2EIModel(
 )
 
 data class E2EIConfigModel(
-    val discoverUrl: String,
+    val discoverUrl: String?,
     val verificationExpirationSeconds: Long
 )
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
@@ -1556,7 +1556,7 @@ class UserSessionScope internal constructor(
         get() = CertificateRevocationListRepositoryDataSource(
             acmeApi = globalScope.unboundNetworkContainer.acmeApi,
             metadataDAO = userStorage.database.metadataDAO,
-            userConfigRepository = userConfigRepository
+            e2eiRepository = e2eiRepository
         )
 
     private val proteusPreKeyRefiller: ProteusPreKeyRefiller

diff --git a/...ommonMain/kotlin/com/wire/kalium/logic/feature/featureConfig/SyncFeatureConfigsUseCase.kt b/...ommonMain/kotlin/com/wire/kalium/logic/feature/featureConfig/SyncFeatureConfigsUseCase.kt
@@ -61,7 +61,7 @@ internal class SyncFeatureConfigsUseCaseImpl(
     private val appLockConfigHandler: AppLockConfigHandler
 ) : SyncFeatureConfigsUseCase {
     override suspend operator fun invoke(): Either<CoreFailure, Unit> =
-        featureConfigRepository.getFeatureConfigs().flatMap {
+        featureConfigRepository.getFeatureConfigs().flatMap { it ->
             // TODO handle other feature flags and after it bump version in [SlowSyncManager.CURRENT_VERSION]
             guestRoomConfigHandler.handle(it.guestRoomLinkModel)
             fileSharingConfigHandler.handle(it.fileSharingModel)
@@ -71,7 +71,7 @@ internal class SyncFeatureConfigsUseCaseImpl(
             conferenceCallingConfigHandler.handle(it.conferenceCallingModel)
             passwordChallengeConfigHandler.handle(it.secondFactorPasswordChallengeModel)
             selfDeletingMessagesConfigHandler.handle(it.selfDeletingMessagesModel)
-            e2EIConfigHandler.handle(it.e2EIModel)
+            it.e2EIModel?.let { e2EIModel -> e2EIConfigHandler.handle(e2EIModel) }
             appLockConfigHandler.handle(it.appLockModel)
             Either.Right(Unit)
         }.onFailure { networkFailure ->

diff --git a/...ommonMain/kotlin/com/wire/kalium/logic/feature/featureConfig/handler/E2EIConfigHandler.kt b/...ommonMain/kotlin/com/wire/kalium/logic/feature/featureConfig/handler/E2EIConfigHandler.kt
@@ -31,8 +31,12 @@ class E2EIConfigHandler(
     private val userConfigRepository: UserConfigRepository,
 ) {
     fun handle(e2eiConfig: E2EIModel): Either<CoreFailure, Unit> {
-        val gracePeriodEnd = DateTimeUtil.currentInstant()
-            .plus(e2eiConfig.config.verificationExpirationSeconds.toDuration(DurationUnit.SECONDS))
+        val gracePeriodEnd = e2eiConfig.config.verificationExpirationSeconds
+            .toDuration(DurationUnit.SECONDS)
+            .let {
+            DateTimeUtil.currentInstant().plus(it)
+        }
+
         userConfigRepository.setE2EISettings(
             E2EISettings(
                 isRequired = e2eiConfig.status == Status.ENABLED,

diff --git a/...monTest/kotlin/com/wire/kalium/logic/data/e2ei/CertificateRevocationListRepositoryTest.kt b/...monTest/kotlin/com/wire/kalium/logic/data/e2ei/CertificateRevocationListRepositoryTest.kt
@@ -115,9 +115,9 @@ class CertificateRevocationListRepositoryTest {
         val metadataDAO = mock(classOf<MetadataDAO>())
 
         @Mock
-        val userConfigRepository = mock(classOf<UserConfigRepository>())
+        val e2eiRepository = mock(classOf<E2EIRepository>())
 
-        fun arrange() = this to CertificateRevocationListRepositoryDataSource(acmeApi, metadataDAO, userConfigRepository)
+        fun arrange() = this to CertificateRevocationListRepositoryDataSource(acmeApi, metadataDAO, e2eiRepository)
 
         suspend fun withEmptyList() = apply {
             given(metadataDAO).coroutine {

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
@@ -25,6 +25,7 @@ import com.wire.kalium.cryptography.MLSClient
 import com.wire.kalium.cryptography.NewAcmeAuthz
 import com.wire.kalium.cryptography.NewAcmeOrder
 import com.wire.kalium.logic.CoreFailure
+import com.wire.kalium.logic.E2EIFailure
 import com.wire.kalium.logic.StorageFailure
 import com.wire.kalium.logic.configuration.E2EISettings
 import com.wire.kalium.logic.configuration.UserConfigRepository
@@ -68,6 +69,7 @@ import io.mockative.thenDoNothing
 import io.mockative.time
 import io.mockative.verify
 import kotlinx.coroutines.test.runTest
+import kotlinx.datetime.Instant
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertIs
@@ -309,7 +311,7 @@ class E2EIRepositoryTest {
         // Given
         val (arrangement, e2eiRepository) = Arrangement()
             .withGetNewAuthzRequestSuccessful()
-            .withSendAuthorizationRequestSucceed(url = RANDOM_URL ,DtoAuthorizationChallengeType.DPoP)
+            .withSendAuthorizationRequestSucceed(url = RANDOM_URL, DtoAuthorizationChallengeType.DPoP)
             .withSendAcmeRequestApiSucceed()
             .withSetAuthzResponseSuccessful()
             .withSendAcmeRequestApiSucceed()
@@ -414,7 +416,7 @@ class E2EIRepositoryTest {
         val expected = Arrangement.AUTHORIZATIONS_RESULT
         // Given
         val (arrangement, e2eiRepository) = Arrangement()
-            .withSendAuthorizationRequestSucceed(url = authorizationsUrls[0],DtoAuthorizationChallengeType.DPoP)
+            .withSendAuthorizationRequestSucceed(url = authorizationsUrls[0], DtoAuthorizationChallengeType.DPoP)
             .withSendAuthorizationRequestSucceed(url = authorizationsUrls[1], DtoAuthorizationChallengeType.OIDC)
             .withGetE2EIClientSuccessful()
             .withGetMLSClientSuccessful()
@@ -926,7 +928,7 @@ class E2EIRepositoryTest {
         // Given
 
         val (arrangement, e2eiRepository) = Arrangement()
-            .withGettingE2EISettingsReturns(Either.Right(E2EI_TEAM_SETTINGS.copy(discoverUrl = RANDOM_URL+"/random/path")))
+            .withGettingE2EISettingsReturns(Either.Right(E2EI_TEAM_SETTINGS.copy(discoverUrl = RANDOM_URL + "/random/path")))
             .withFetchAcmeTrustAnchorsApiSucceed()
             .withGetMLSClientSuccessful()
             .withRegisterTrustAnchors()
@@ -953,6 +955,52 @@ class E2EIRepositoryTest {
             .with(eq(Arrangement.RANDOM_BYTE_ARRAY.decodeToString()))
             .wasInvoked(once)
     }
+
+    @Test
+    fun givenE2EIIsDisabled_whenCallingDiscoveryUrl_thenItFailWithDisabled() {
+        val (arrangement, e2eiRepository) = Arrangement()
+            .withGettingE2EISettingsReturns(Either.Right(E2EISettings(false, null, Instant.DISTANT_FUTURE)))
+            .arrange()
+
+        e2eiRepository.discoveryUrl().shouldFail {
+            assertIs<E2EIFailure.Disabled>(it)
+        }
+
+        verify(arrangement.userConfigRepository)
+            .function(arrangement.userConfigRepository::getE2EISettings)
+            .wasInvoked(once)
+    }
+
+    @Test
+    fun givenE2EIIsEnabledAndDiscoveryUrlIsNull_whenCallingDiscoveryUrl_thenItFailWithMissingDiscoveryUrl() {
+        val (arrangement, e2eiRepository) = Arrangement()
+            .withGettingE2EISettingsReturns(Either.Right(E2EISettings(true, null, Instant.DISTANT_FUTURE)))
+            .arrange()
+
+        e2eiRepository.discoveryUrl().shouldFail {
+            assertIs<E2EIFailure.MissingDiscoveryUrl>(it)
+        }
+
+        verify(arrangement.userConfigRepository)
+            .function(arrangement.userConfigRepository::getE2EISettings)
+            .wasInvoked(once)
+    }
+
+    @Test
+    fun givenE2EIIsEnabledAndDiscoveryUrlIsNotNull_whenCallingDiscoveryUrl_thenItSucceed() {
+        val (arrangement, e2eiRepository) = Arrangement()
+            .withGettingE2EISettingsReturns(Either.Right(E2EISettings(true, RANDOM_URL, Instant.DISTANT_FUTURE)))
+            .arrange()
+
+        e2eiRepository.discoveryUrl().shouldSucceed {
+            assertEquals(RANDOM_URL, it)
+        }
+
+        verify(arrangement.userConfigRepository)
+            .function(arrangement.userConfigRepository::getE2EISettings)
+            .wasInvoked(once)
+    }
+
     private class Arrangement {
 
         fun withGetE2EIClientSuccessful() = apply {

diff --git a/...in/com/wire/kalium/network/api/base/authenticated/featureConfigs/FeatureConfigResponse.kt b/...in/com/wire/kalium/network/api/base/authenticated/featureConfigs/FeatureConfigResponse.kt
@@ -117,7 +117,7 @@ data class E2EIConfigDTO(
     @SerialName("acmeDiscoveryUrl")
     val url: String?,
     @SerialName("verificationExpiration")
-    val verificationExpirationSeconds: Long = 0L
+    val verificationExpirationSeconds: Long
 )
 
 @OptIn(ExperimentalSerializationApi::class)

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
@@ -203,7 +203,7 @@ data class TeamSettingsSelfDeletionStatusEntity(
 @Serializable
 data class E2EISettingsEntity(
     @SerialName("status") val status: Boolean,
-    @SerialName("discoverUrl") val discoverUrl: String,
+    @SerialName("discoverUrl") val discoverUrl: String?,
     @SerialName("gracePeriodEndMs") val gracePeriodEndMs: Long?,
 )