Skip to content

Commit

Permalink
chore: address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@ohassine
ohassine committed Jan 25, 2024
1 parent 1560656 commit 2e06132
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 313 deletions.
40 changes: 3 additions & 37 deletions ...e/kalium/logic/data/e2ei/CrlRepository.kt → ...ei/CertificateRevocationListRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,68 +18,35 @@
package com.wire.kalium.logic.data.e2ei

import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.StorageFailure
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
import com.wire.kalium.logic.wrapStorageRequest
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.Url
import io.ktor.http.protocolWithAuthority
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map
import kotlinx.datetime.Instant

interface CrlRepository {
interface CertificateRevocationListRepository {

/**
* Returns CRLs with expiration time.
*
* @return the [CRLUrlExpirationList] representing a list of CRLs with expiration time.
*/
suspend fun getCRLs(): CRLUrlExpirationList?

/**
* Observes the last CRL check instant.
*
* @return A [Flow] of [Instant] objects representing the last CRL check instant.
* It emits `null` if no CRL check has occurred.
*/
suspend fun lastCrlCheckInstantFlow(): Flow<Instant?>

/**
* Sets the last CRL check date.
*
* @param instant The instant representing the date and time of the last CRL check.
* @return Either a [StorageFailure] if the operation fails, or [Unit] if successful.
*/
suspend fun setLastCRLCheckInstant(instant: Instant): Either<StorageFailure, Unit>

suspend fun addOrUpdateCRL(url: String, timestamp: ULong)
suspend fun getCurrentClientCrlUrl(): Either<CoreFailure, String>
suspend fun getClientDomainCRL(url: String): Either<CoreFailure, ByteArray>
}

internal class CrlRepositoryDataSource(
internal class CertificateRevocationListRepositoryDataSource(
private val acmeApi: ACMEApi,
private val metadataDAO: MetadataDAO,
private val userConfigRepository: UserConfigRepository
) : CrlRepository {

override suspend fun lastCrlCheckInstantFlow(): Flow<Instant?> =
metadataDAO.valueByKeyFlow(CRL_CHECK_INSTANT_KEY).map { instant ->
instant?.let { Instant.parse(it) }
}

override suspend fun setLastCRLCheckInstant(instant: Instant): Either<StorageFailure, Unit> =
wrapStorageRequest {
metadataDAO.insertValue(instant.toString(), CRL_CHECK_INSTANT_KEY)
}

) : CertificateRevocationListRepository {
override suspend fun getCRLs(): CRLUrlExpirationList? =
metadataDAO.getSerializable(CRL_LIST_KEY, CRLUrlExpirationList.serializer())

Expand Down Expand Up @@ -125,6 +92,5 @@ internal class CrlRepositoryDataSource(

companion object {
const val CRL_LIST_KEY = "crl_list_key"
const val CRL_CHECK_INSTANT_KEY = "crl_check_instant_key"
}
}
22 changes: 11 additions & 11 deletions logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ import com.wire.kalium.logic.data.conversation.ProposalTimer
import com.wire.kalium.logic.data.conversation.SubconversationRepositoryImpl
import com.wire.kalium.logic.data.conversation.UpdateKeyingMaterialThresholdProvider
import com.wire.kalium.logic.data.conversation.UpdateKeyingMaterialThresholdProviderImpl
import com.wire.kalium.logic.data.e2ei.CrlRepository
import com.wire.kalium.logic.data.e2ei.CrlRepositoryDataSource
import com.wire.kalium.logic.data.e2ei.CertificateRevocationListRepository
import com.wire.kalium.logic.data.e2ei.CertificateRevocationListRepositoryDataSource
import com.wire.kalium.logic.data.e2ei.E2EIRepository
import com.wire.kalium.logic.data.e2ei.E2EIRepositoryImpl
import com.wire.kalium.logic.data.event.EventDataSource
Expand Down Expand Up @@ -207,8 +207,8 @@ import com.wire.kalium.logic.feature.conversation.mls.OneOnOneResolverImpl
import com.wire.kalium.logic.feature.debug.DebugScope
import com.wire.kalium.logic.feature.e2ei.ACMECertificatesSyncWorker
import com.wire.kalium.logic.feature.e2ei.ACMECertificatesSyncWorkerImpl
import com.wire.kalium.logic.feature.e2ei.CheckCrlWorker
import com.wire.kalium.logic.feature.e2ei.CheckCrlWorkerImpl
import com.wire.kalium.logic.feature.e2ei.CertificateRevocationListCheckWorker
import com.wire.kalium.logic.feature.e2ei.CertificateRevocationListCheckWorkerImpl
import com.wire.kalium.logic.feature.e2ei.usecase.CheckRevocationListUseCase
import com.wire.kalium.logic.feature.e2ei.usecase.CheckRevocationListUseCaseImpl
import com.wire.kalium.logic.feature.e2ei.usecase.EnrollE2EIUseCase
Expand Down Expand Up @@ -1509,8 +1509,8 @@ class UserSessionScope internal constructor(
userStorage.database.clientDAO,
userStorage.database.metadataDAO,
)
private val crlRepository: CrlRepository
get() = CrlRepositoryDataSource(
private val certificateRevocationListRepository: CertificateRevocationListRepository
get() = CertificateRevocationListRepositoryDataSource(
acmeApi = globalScope.unboundNetworkContainer.acmeApi,
metadataDAO = userStorage.database.metadataDAO,
userConfigRepository = userConfigRepository
Expand All @@ -1527,9 +1527,9 @@ class UserSessionScope internal constructor(
)
}

private val checkCrlWorker: CheckCrlWorker by lazy {
CheckCrlWorkerImpl(
crlRepository = crlRepository,
private val certificateRevocationListCheckWorker: CertificateRevocationListCheckWorker by lazy {
CertificateRevocationListCheckWorkerImpl(
certificateRevocationListRepository = certificateRevocationListRepository,
incrementalSyncRepository = incrementalSyncRepository,
checkRevocationList = checkRevocationList,
)
Expand Down Expand Up @@ -1888,7 +1888,7 @@ class UserSessionScope internal constructor(

private val checkRevocationList: CheckRevocationListUseCase
get() = CheckRevocationListUseCaseImpl(
crlRepository = crlRepository,
certificateRevocationListRepository = certificateRevocationListRepository,
currentClientIdProvider = clientIdProvider,
mlsClientProvider = mlsClientProvider,
mLSConversationsVerificationStatusesHandler = mlsConversationsVerificationStatusesHandler
Expand Down Expand Up @@ -1927,7 +1927,7 @@ class UserSessionScope internal constructor(
}

launch {
checkCrlWorker.execute()
certificateRevocationListCheckWorker.execute()
}

launch {
Expand Down
50 changes: 13 additions & 37 deletions ...lium/logic/feature/e2ei/CheckCrlWorker.kt → ...i/CertificateRevocationListCheckWorker.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,74 +17,50 @@
*/
package com.wire.kalium.logic.feature.e2ei

import com.wire.kalium.logic.data.e2ei.CrlRepository
import com.wire.kalium.logic.data.e2ei.CertificateRevocationListRepository
import com.wire.kalium.logic.data.sync.IncrementalSyncRepository
import com.wire.kalium.logic.data.sync.IncrementalSyncStatus
import com.wire.kalium.logic.feature.e2ei.usecase.CheckRevocationListUseCase
import com.wire.kalium.logic.functional.map
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.collectLatest
import com.wire.kalium.logic.kaliumLogger
import kotlinx.coroutines.flow.filter
import kotlinx.datetime.Clock
import kotlin.time.Duration
import kotlin.time.Duration.Companion.hours

/**
* Represents an interface for a CheckCrlWorker,
* which is should keep the [CrlRepository] healthy.
* This worker will wait until the sync is done and then check the CRLs if needed.
*
* It will wait until the incremental sync is live and then check the CRLs if needed.
*/
internal interface CheckCrlWorker {
internal interface CertificateRevocationListCheckWorker {
suspend fun execute()
}

/**
* Base implementation of [CheckCrlWorker].
* @param crlRepository The CRL repository.
* Base implementation of [CertificateRevocationListCheckWorker].
* @param certificateRevocationListRepository The CRL repository.
* @param incrementalSyncRepository The incremental sync repository.
* @param checkRevocationList The check revocation list use case.
* @param minIntervalBetweenRefills The minimum interval between CRL checks.
*
*/
internal class CheckCrlWorkerImpl(
private val crlRepository: CrlRepository,
internal class CertificateRevocationListCheckWorkerImpl(
private val certificateRevocationListRepository: CertificateRevocationListRepository,
private val incrementalSyncRepository: IncrementalSyncRepository,
private val checkRevocationList: CheckRevocationListUseCase,
private val minIntervalBetweenRefills: Duration = MIN_INTERVAL_BETWEEN_REFILLS
) : CheckCrlWorker {
private val checkRevocationList: CheckRevocationListUseCase
) : CertificateRevocationListCheckWorker {

/**
* Check the CRLs and update the expiration time if needed.
*/
override suspend fun execute() {
crlRepository.lastCrlCheckInstantFlow().collectLatest { lastCheck ->
val now = Clock.System.now()
val nextCheckTime = lastCheck?.plus(minIntervalBetweenRefills) ?: now
val delayUntilNextCheck = nextCheckTime - now
delay(delayUntilNextCheck)
waitUntilLiveAndCheckCRLs()
crlRepository.setLastCRLCheckInstant(Clock.System.now())
}
}

private suspend fun waitUntilLiveAndCheckCRLs() {
incrementalSyncRepository.incrementalSyncState
.filter { it is IncrementalSyncStatus.Live }
.collect {
crlRepository.getCRLs()?.cRLWithExpirationList?.forEach { crl ->
kaliumLogger.i("Checking certificate revocation list (CRL)..")
certificateRevocationListRepository.getCRLs()?.cRLWithExpirationList?.forEach { crl ->
if (crl.expiration < Clock.System.now().epochSeconds.toULong()) {
checkRevocationList(crl.url).map { newExpirationTime ->
newExpirationTime?.let {
crlRepository.addOrUpdateCRL(crl.url, it)
certificateRevocationListRepository.addOrUpdateCRL(crl.url, it)
}
}
}
}
}
}

private companion object {
val MIN_INTERVAL_BETWEEN_REFILLS = 24.hours
}
}
6 changes: 3 additions & 3 deletions ...ommonMain/kotlin/com/wire/kalium/logic/feature/e2ei/usecase/CheckRevocationListUseCase.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package com.wire.kalium.logic.feature.e2ei.usecase

import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.data.client.MLSClientProvider
import com.wire.kalium.logic.data.e2ei.CrlRepository
import com.wire.kalium.logic.data.e2ei.CertificateRevocationListRepository
import com.wire.kalium.logic.data.id.CurrentClientIdProvider
import com.wire.kalium.logic.feature.conversation.MLSConversationsVerificationStatusesHandler
import com.wire.kalium.logic.functional.Either
Expand All @@ -34,13 +34,13 @@ interface CheckRevocationListUseCase {
}

internal class CheckRevocationListUseCaseImpl(
private val crlRepository: CrlRepository,
private val certificateRevocationListRepository: CertificateRevocationListRepository,
private val currentClientIdProvider: CurrentClientIdProvider,
private val mlsClientProvider: MLSClientProvider,
private val mLSConversationsVerificationStatusesHandler: MLSConversationsVerificationStatusesHandler
) : CheckRevocationListUseCase {
override suspend fun invoke(url: String): Either<CoreFailure, ULong?> {
return crlRepository.getClientDomainCRL(url).flatMap {
return certificateRevocationListRepository.getClientDomainCRL(url).flatMap {
currentClientIdProvider().flatMap { clientId ->
mlsClientProvider.getMLSClient(clientId).map { mlsClient ->
mlsClient.registerCrl(url, it).run {
Expand Down
6 changes: 3 additions & 3 deletions ...lium/logic/data/e2ei/CrlRepositoryTest.kt → ...ertificateRevocationListRepositoryTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package com.wire.kalium.logic.data.e2ei

import com.wire.kalium.logic.configuration.UserConfigRepository
import com.wire.kalium.logic.data.e2ei.CrlRepositoryDataSource.Companion.CRL_LIST_KEY
import com.wire.kalium.logic.data.e2ei.CertificateRevocationListRepositoryDataSource.Companion.CRL_LIST_KEY
import com.wire.kalium.network.api.base.unbound.acme.ACMEApi
import com.wire.kalium.persistence.config.CRLWithExpiration
import com.wire.kalium.persistence.config.CRLUrlExpirationList
Expand All @@ -32,7 +32,7 @@ import io.mockative.verify
import kotlinx.coroutines.test.runTest
import kotlin.test.Test

class CrlRepositoryTest {
class CertificateRevocationListRepositoryTest {

@Test
fun givenAnEmptyStoredList_whenUpdatingCRLs_thenAddNewCRL() = runTest {
Expand Down Expand Up @@ -101,7 +101,7 @@ class CrlRepositoryTest {
@Mock
val userConfigRepository = mock(classOf<UserConfigRepository>())

fun arrange() = this to CrlRepositoryDataSource(acmeApi, metadataDAO, userConfigRepository)
fun arrange() = this to CertificateRevocationListRepositoryDataSource(acmeApi, metadataDAO, userConfigRepository)

suspend fun withEmptyList() = apply {
given(metadataDAO).coroutine {
Expand Down
Loading

0 comments on commit 2e06132

Please sign in to comment.