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.

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: OtherUser devices: wrong MLS data [WPB-8908] #3040

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,12 @@ private fun DeviceItemTexts(
)
if (shouldShowVerifyLabel) {
if (shouldShowE2EIInfo) {
MLSVerificationIcon(device.e2eiCertificateStatus)
MLSVerificationIcon(device.e2eiCertificate?.status)
}
Spacer(modifier = Modifier.width(MaterialTheme.wireDimensions.spacing8x))
if (device.isVerifiedProteus && !isCurrentClient) ProteusVerifiedIcon(
Modifier
.wrapContentWidth()
.align(Alignment.CenterVertically))
Modifier.wrapContentWidth().align(Alignment.CenterVertically)
)
}
}

Expand All @@ -206,15 +205,15 @@ private fun DeviceItemTexts(

Spacer(modifier = Modifier.height(MaterialTheme.wireDimensions.removeDeviceItemTitleVerticalPadding))

device.mlsPublicKeys?.values?.firstOrNull()?.let { mlsThumbprint ->
device.e2eiCertificate?.let { certificate ->
Text(
style = MaterialTheme.wireTypography.subline01,
color = MaterialTheme.wireColorScheme.labelText,
maxLines = 1,
overflow = TextOverflow.Ellipsis,
text = stringResource(
R.string.remove_device_mls_thumbprint_label,
mlsThumbprint.formatAsFingerPrint()
certificate.thumbprint.formatAsFingerPrint()
),
modifier = Modifier
.fillMaxWidth()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import com.wire.android.R
import com.wire.android.util.ui.UIText
import com.wire.kalium.logic.data.client.Client
import com.wire.kalium.logic.data.conversation.ClientId
import com.wire.kalium.logic.feature.e2ei.CertificateStatus
import com.wire.kalium.logic.feature.e2ei.E2eiCertificate
import com.wire.kalium.logic.util.inWholeWeeks
import com.wire.kalium.util.DateTimeUtil.toIsoDateTimeString
import kotlinx.datetime.Clock
Expand All @@ -38,18 +38,16 @@ data class Device(
val lastActiveInWholeWeeks: Int? = null,
val isValid: Boolean = true,
val isVerifiedProteus: Boolean = false,
val mlsPublicKeys: Map<String, String>? = null,
val e2eiCertificateStatus: CertificateStatus? = null
val e2eiCertificate: E2eiCertificate? = null
) {
constructor(client: Client, e2eiCertificateStatus: CertificateStatus? = null) : this(
constructor(client: Client, e2eiCertificate: E2eiCertificate? = null) : this(
name = client.displayName(),
clientId = client.id,
registrationTime = client.registrationTime?.toIsoDateTimeString(),
lastActiveInWholeWeeks = client.lastActiveInWholeWeeks(),
isValid = client.isValid,
isVerifiedProteus = client.isVerified,
mlsPublicKeys = client.mlsPublicKeys,
e2eiCertificateStatus = e2eiCertificateStatus
e2eiCertificate = e2eiCertificate
)

fun updateFromClient(client: Client): Device = copy(
Expand All @@ -59,11 +57,11 @@ data class Device(
lastActiveInWholeWeeks = client.lastActiveInWholeWeeks(),
isValid = client.isValid,
isVerifiedProteus = client.isVerified,
mlsPublicKeys = client.mlsPublicKeys,
e2eiCertificate = null,
)

fun updateE2EICertificateStatus(e2eiCertificateStatus: CertificateStatus): Device = copy(
e2eiCertificateStatus = e2eiCertificateStatus
fun updateE2EICertificate(e2eiCertificate: E2eiCertificate): Device = copy(
e2eiCertificate = e2eiCertificate
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,11 @@ import com.wire.android.util.extension.formatAsString
import com.wire.android.util.ui.UIText
import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.data.conversation.ClientId
import com.wire.kalium.logic.feature.e2ei.CertificateStatus
import com.wire.kalium.logic.feature.e2ei.E2eiCertificate
import com.wire.kalium.logic.feature.e2ei.usecase.E2EIEnrollmentResult
import com.wire.kalium.logic.functional.Either
import kotlinx.datetime.Instant

@RootNavGraph
@Destination(
Expand Down Expand Up @@ -127,6 +129,7 @@ fun DeviceDetailsScreen(
)
}
}

@Suppress("ComplexMethod")
@Composable
fun DeviceDetailsContent(
Expand Down Expand Up @@ -187,9 +190,9 @@ fun DeviceDetailsContent(
.background(MaterialTheme.wireColorScheme.surface)
) {

state.device.mlsPublicKeys?.forEach { (mlsProtocolType, mlsThumbprint) ->
state.device.e2eiCertificate?.let { certificate ->
item {
DeviceMLSSignatureItem(mlsThumbprint, mlsProtocolType, screenState::copyMessage)
DeviceMLSSignatureItem(certificate.thumbprint, screenState::copyMessage)
Divider(color = MaterialTheme.wireColorScheme.background)
}
}
Expand Down Expand Up @@ -323,7 +326,7 @@ private fun DeviceDetailsTopBar(
)

if (shouldShowE2EIInfo) {
MLSVerificationIcon(device.e2eiCertificateStatus)
MLSVerificationIcon(device.e2eiCertificate?.status)
}

if (!isCurrentDevice && device.isVerifiedProteus) {
Expand Down Expand Up @@ -373,17 +376,9 @@ fun DeviceKeyFingerprintItem(
@Composable
fun DeviceMLSSignatureItem(
mlsThumbprint: String,
mlsProtocolType: String,
onCopy: (String) -> Unit
) {

FolderHeader(
name = stringResource(id = R.string.label_mls_signature, mlsProtocolType).uppercase(),
modifier = Modifier
.background(MaterialTheme.wireColorScheme.background)
.fillMaxWidth()
)

DeviceDetailSectionContent(
stringResource(id = R.string.label_mls_thumbprint),
sectionText = mlsThumbprint.formatAsFingerPrint(),
Expand Down Expand Up @@ -578,7 +573,14 @@ fun PreviewDeviceDetailsScreen() {
clientId = ClientId(""),
name = UIText.DynamicString("My Device"),
registrationTime = "2022-03-24T18:02:30.360Z",
mlsPublicKeys = mapOf("Ed25519" to "lekvmrlkgvnrelkmvrlgkvlknrgb0348gi34t09gj34v034ithjoievw")
e2eiCertificate = E2eiCertificate(
"handler",
CertificateStatus.VALID,
"serial",
"date",
"Thumbprint",
Instant.DISTANT_FUTURE
)
),
isCurrentDevice = false
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class DeviceDetailsViewModel @Inject constructor(
isE2eiCertificateActivated = true,
e2eiCertificate = certificate.certificate,
isLoadingCertificate = false,
device = state.device.updateE2EICertificateStatus(certificate.certificate.status)
device = state.device.updateE2EICertificate(certificate.certificate)
borichellow marked this conversation as resolved.
Show resolved Hide resolved
)
} else {
state.copy(isE2eiCertificateActivated = false, isLoadingCertificate = false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ class SelfDevicesViewModel @Inject constructor(
isLoadingClientsList = false,
currentDevice = result.clients
.firstOrNull { it.id == currentClientId }
?.let { Device(it, e2eiCertificates[it.id.value]?.status) },
?.let { Device(it, e2eiCertificates[it.id.value]) },
deviceList = result.clients
.filter { it.id != currentClientId }
.map { Device(it, e2eiCertificates[it.id.value]?.status) }
.map { Device(it, e2eiCertificates[it.id.value]) }
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private fun OtherUserDevicesContent(
onClickAction = onDeviceClick,
icon = Icons.Filled.ChevronRight.Icon(),
shouldShowVerifyLabel = true,
shouldShowE2EIInfo = item.e2eiCertificateStatus != null
shouldShowE2EIInfo = item.e2eiCertificate != null
)
if (index < otherUserDevices.lastIndex) WireDivider()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class OtherUserProfileScreenViewModel @Inject constructor(

is ObserveClientsByUserIdUseCase.Result.Success -> {
state = state.copy(otherUserDevices = it.clients.map { item ->
Device(item, e2eiCertificates[item.id.value]?.status)
Device(item, e2eiCertificates[item.id.value])
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,15 @@ import com.wire.kalium.logic.feature.client.GetClientDetailsResult
import com.wire.kalium.logic.feature.client.ObserveClientDetailsUseCase
import com.wire.kalium.logic.feature.client.Result
import com.wire.kalium.logic.feature.client.UpdateClientVerificationStatusUseCase
import com.wire.kalium.logic.feature.e2ei.CertificateStatus
import com.wire.kalium.logic.feature.e2ei.E2eiCertificate
import com.wire.kalium.logic.feature.e2ei.usecase.GetE2EICertificateUseCaseResult
import com.wire.kalium.logic.feature.e2ei.usecase.GetE2eiCertificateUseCase
import com.wire.kalium.logic.feature.user.GetUserInfoResult
import com.wire.kalium.logic.feature.user.IsE2EIEnabledUseCase
import com.wire.kalium.logic.feature.user.IsPasswordRequiredUseCase
import com.wire.kalium.logic.feature.user.ObserveUserInfoUseCase
import com.wire.kalium.util.DateTimeUtil
import io.mockk.Called
import io.mockk.MockKAnnotations
import io.mockk.coEvery
Expand All @@ -60,6 +63,7 @@ import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import kotlin.time.Duration.Companion.days

@OptIn(ExperimentalCoroutinesApi::class)
@ExtendWith(CoroutineTestExtension::class)
Expand Down Expand Up @@ -281,6 +285,30 @@ class DeviceDetailsViewModelTest {
assertTrue(viewModel.state.startGettingE2EICertificate)
}


@Test
fun `given a client with E2EI certificate, when fetching details, then returns device information`() {
val certificate = E2eiCertificate(
userHandle = "userHandle",
serialNumber = "serialNumber",
certificateDetail = "certificateDetail",
status = CertificateStatus.VALID,
thumbprint = "thumbprint",
endAt = DateTimeUtil.currentInstant().plus(1.days)
)
runTest {
// given
val (_, viewModel) = Arrangement()
.withRequiredMockSetup()
.withClientDetailsResult(GetClientDetailsResult.Success(TestClient.CLIENT, true))
.withE2eiCertificate(GetE2EICertificateUseCaseResult.Success(certificate))
.arrange()

// then
assertEquals(certificate, viewModel.state.device.e2eiCertificate)
}
}

private class Arrangement {

@MockK
Expand Down Expand Up @@ -370,6 +398,10 @@ class DeviceDetailsViewModelTest {
)
}

fun withE2eiCertificate(result: GetE2EICertificateUseCaseResult) = apply {
coEvery { getE2eiCertificate(any()) } returns result
}

fun arrange() = this to viewModel

companion object {
Expand Down
Loading