Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Sending of message in empty MLS group #WPB-14747 #3176

Merged
merged 4 commits into from
Dec 13, 2024
Merged

fix: Sending of message in empty MLS group #WPB-14747 #3176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
40e523e
fix: Sending of message in empty MLS group
borichellow Dec 13, 2024
9f59cd3
Merge branch 'release/candidate' into fix/sending_of_messages_in_empt…
borichellow Dec 13, 2024
c7767d7
Fixed code style
borichellow Dec 13, 2024
170c532
Fixed tests
borichellow Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 38 additions & 37 deletions ...rc/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import com.wire.kalium.logic.StorageFailure
import com.wire.kalium.logic.data.client.MLSClientProvider
import com.wire.kalium.logic.data.conversation.mls.MLSAdditionResult
import com.wire.kalium.logic.data.e2ei.CertificateRevocationListRepository
import com.wire.kalium.logic.data.e2ei.RevocationListChecker
import com.wire.kalium.logic.data.event.Event
import com.wire.kalium.logic.data.event.EventDeliveryInfo
import com.wire.kalium.logic.data.id.ConversationId
Expand All @@ -45,12 +46,11 @@ import com.wire.kalium.logic.data.id.toModel
import com.wire.kalium.logic.data.keypackage.KeyPackageLimitsProvider
import com.wire.kalium.logic.data.keypackage.KeyPackageRepository
import com.wire.kalium.logic.data.mls.CipherSuite
import com.wire.kalium.logic.data.mls.MLSPublicKeys
import com.wire.kalium.logic.data.mlspublickeys.MLSPublicKeysRepository
import com.wire.kalium.logic.data.mlspublickeys.getRemovalKey
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.di.MapperProvider
import com.wire.kalium.logic.data.e2ei.RevocationListChecker
import com.wire.kalium.logic.data.mls.MLSPublicKeys
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.functional.flatMap
import com.wire.kalium.logic.functional.flatMapLeft
Expand All @@ -68,9 +68,9 @@ import com.wire.kalium.logic.sync.incremental.EventSource
import com.wire.kalium.logic.wrapApiRequest
import com.wire.kalium.logic.wrapMLSRequest
import com.wire.kalium.logic.wrapStorageRequest
import com.wire.kalium.network.api.authenticated.notification.EventContentDTO
import com.wire.kalium.network.api.base.authenticated.client.ClientApi
import com.wire.kalium.network.api.base.authenticated.message.MLSMessageApi
import com.wire.kalium.network.api.authenticated.notification.EventContentDTO
import com.wire.kalium.network.exceptions.KaliumException
import com.wire.kalium.network.exceptions.isMlsClientMismatch
import com.wire.kalium.network.exceptions.isMlsCommitMissingReferences
Expand Down Expand Up @@ -481,7 +481,7 @@ internal class MLSConversationDataSource(
val keyPackages = result.successfullyFetchedKeyPackages
val clientKeyPackageList = keyPackages.map { it.keyPackage.decodeBase64Bytes() }
wrapMLSRequest {
if (userIdList.isEmpty()) {
if (clientKeyPackageList.isEmpty()) {
// We are creating a group with only our self client which technically
// doesn't need be added with a commit, but our backend API requires one,
// so we create a commit by updating our key material.
Expand Down Expand Up @@ -566,6 +566,7 @@ internal class MLSConversationDataSource(

keys.flatMap { externalSenders ->
establishMLSGroup(
mlsClient = mlsClient,
groupID = groupID,
members = members,
externalSenders = externalSenders,
Expand All @@ -583,6 +584,7 @@ internal class MLSConversationDataSource(
conversationDAO.getMLSGroupIdByConversationId(parentId.toDao())?.let { parentGroupId ->
val externalSenderKey = mlsClient.getExternalSenders(GroupID(parentGroupId).toCrypto())
establishMLSGroup(
mlsClient = mlsClient,
groupID = groupID,
members = emptyList(),
externalSenders = externalSenderKey.value,
Expand All @@ -593,45 +595,44 @@ internal class MLSConversationDataSource(
}

private suspend fun establishMLSGroup(
mlsClient: MLSClient,
groupID: GroupID,
members: List<UserId>,
externalSenders: ByteArray,
allowPartialMemberList: Boolean = false,
): Either<CoreFailure, MLSAdditionResult> = withContext(serialDispatcher) {
kaliumLogger.d("establish MLS group: $groupID")
mlsClientProvider.getMLSClient().flatMap { mlsClient ->
wrapMLSRequest {
mlsClient.createConversation(
idMapper.toCryptoModel(groupID),
externalSenders
)
}.flatMapLeft {
if (it is MLSFailure.ConversationAlreadyExists) {
Either.Right(Unit)
} else {
Either.Left(it)
}
}.flatMap {
internalAddMemberToMLSGroup(
groupID = groupID,
userIdList = members,
retryOnStaleMessage = false,
allowPartialMemberList = allowPartialMemberList,
cipherSuite = CipherSuite.fromTag(mlsClient.getDefaultCipherSuite())
).onFailure {
wrapMLSRequest {
mlsClient.wipeConversation(groupID.toCrypto())
}
wrapMLSRequest {
mlsClient.createConversation(
idMapper.toCryptoModel(groupID),
externalSenders
)
}.flatMapLeft {
if (it is MLSFailure.ConversationAlreadyExists) {
Either.Right(Unit)
} else {
Either.Left(it)
}
}.flatMap {
internalAddMemberToMLSGroup(
groupID = groupID,
userIdList = members,
retryOnStaleMessage = false,
allowPartialMemberList = allowPartialMemberList,
cipherSuite = CipherSuite.fromTag(mlsClient.getDefaultCipherSuite())
).onFailure {
wrapMLSRequest {
mlsClient.wipeConversation(groupID.toCrypto())
}
}.flatMap { additionResult ->
wrapStorageRequest {
conversationDAO.updateMlsGroupStateAndCipherSuite(
ConversationEntity.GroupState.ESTABLISHED,
ConversationEntity.CipherSuite.fromTag(mlsClient.getDefaultCipherSuite().toInt()),
idMapper.toGroupIDEntity(groupID)
)
}.map { additionResult }
}
}.flatMap { additionResult ->
wrapStorageRequest {
conversationDAO.updateMlsGroupStateAndCipherSuite(
ConversationEntity.GroupState.ESTABLISHED,
ConversationEntity.CipherSuite.fromTag(mlsClient.getDefaultCipherSuite().toInt()),
idMapper.toGroupIDEntity(groupID)
)
}.map { additionResult }
}
}

Expand All @@ -656,8 +657,8 @@ internal class MLSConversationDataSource(
keyPackageRepository
.replaceKeyPackages(clientId, rotateBundle.newKeyPackages, CipherSuite.fromTag(mlsClient.getDefaultCipherSuite()))
.flatMapLeft {
return E2EIFailure.RotationAndMigration(it).left()
}
return E2EIFailure.RotationAndMigration(it).left()
}
}
kaliumLogger.w("send migration commits after key rotations")
kaliumLogger.w("rotate bundles: ${rotateBundle.commits.size}")
Expand Down
17 changes: 9 additions & 8 deletions ...ommonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ import io.mockative.matches
import io.mockative.mock
import io.mockative.once
import io.mockative.twice
import io.mockative.verify
import kotlinx.coroutines.async
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.first
Expand Down Expand Up @@ -304,13 +303,15 @@ class MLSConversationRepositoryTest {
coVerify {
arrangement.mlsClient.createConversation(
groupId = eq(Arrangement.RAW_GROUP_ID),
externalSenders = any())
externalSenders = any()
)
}.wasInvoked(once)

coVerify {
arrangement.mlsClient.addMember(
groupId = eq(Arrangement.RAW_GROUP_ID),
membersKeyPackages = any())
membersKeyPackages = any()
)
}.wasInvoked(once)

coVerify {
Expand Down Expand Up @@ -1512,7 +1513,7 @@ class MLSConversationRepositoryTest {

val (arrangement, mlsConversationRepository) = Arrangement(testKaliumDispatcher)
.withCommitPendingProposalsReturningNothing()
.withClaimKeyPackagesSuccessful()
.withClaimKeyPackagesSuccessful(emptyList()) // empty cause members is empty in case of establishMLSSubConversationGroup
.withGetMLSClientSuccessful()
.withGetMLSGroupIdByConversationIdReturns(Arrangement.GROUP_ID.value)
.withGetExternalSenderKeySuccessful()
Expand Down Expand Up @@ -1925,10 +1926,10 @@ class MLSConversationRepositoryTest {
"user_handle",
"wire.com"
),
"User Test",
"domain.com",
"certificate",
serialNumber = "serialNumber",
"User Test",
"domain.com",
"certificate",
serialNumber = "serialNumber",
notAfter = 1899105093,
notBefore = 1899205093
)
Expand Down
Loading