From f0feacf12c177b9ebe738eace21d98711b1e2034 Mon Sep 17 00:00:00 2001 From: Mohamad Jaara Date: Tue, 10 Sep 2024 17:57:30 +0200 Subject: [PATCH 1/2] Commit with unresolved merge conflicts --- .../kalium/logic/data/message/AssetContent.kt | 2 +- .../feature/asset/GetMessageAssetUseCase.kt | 9 ++ .../asset/ScheduleNewAssetMessageUseCase.kt | 7 +- .../asset/ValidateAssetFileTypeUseCase.kt | 89 +++++++++++-- .../receiver/asset/AssetMessageHandler.kt | 102 ++++++++++++--- .../ScheduleNewAssetMessageUseCaseTest.kt | 12 ++ .../asset/ValidateAssetFileTypeUseCaseTest.kt | 42 +++++-- .../receiver/asset/AssetMessageHandlerTest.kt | 119 ++++++++++++++++++ 8 files changed, 339 insertions(+), 43 deletions(-) diff --git a/data/src/commonMain/kotlin/com/wire/kalium/logic/data/message/AssetContent.kt b/data/src/commonMain/kotlin/com/wire/kalium/logic/data/message/AssetContent.kt index 1c77a460c34..3b1fe3cde5a 100644 --- a/data/src/commonMain/kotlin/com/wire/kalium/logic/data/message/AssetContent.kt +++ b/data/src/commonMain/kotlin/com/wire/kalium/logic/data/message/AssetContent.kt @@ -34,7 +34,7 @@ data class AssetContent( } // We should not display Preview Assets (assets w/o valid encryption keys sent by Mac/Web clients) unless they include image metadata - val shouldBeDisplayed = !isPreviewMessage || hasValidImageMetadata + val isAssetDataComplete = !isPreviewMessage || hasValidImageMetadata sealed class AssetMetadata { data class Image(val width: Int, val height: Int) : AssetMetadata() diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCase.kt index 08639c7cfb2..4ba441f7d45 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCase.kt @@ -79,7 +79,16 @@ internal class GetMessageAssetUseCaseImpl( }, { message -> when (val content = message.content) { is MessageContent.Asset -> { +<<<<<<< HEAD // TODO isIncompleteImage should be used here for incomplete messages +======= + val assetDownloadStatus = content.value.downloadStatus + val assetUploadStatus = content.value.uploadStatus + val wasDownloaded: Boolean = assetDownloadStatus == SAVED_INTERNALLY || assetDownloadStatus == SAVED_EXTERNALLY + // assets uploaded by other clients have upload status NOT_UPLOADED + val alreadyUploaded: Boolean = (assetUploadStatus == NOT_UPLOADED && content.value.isAssetDataComplete) + || assetUploadStatus == UPLOADED +>>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) val assetMetadata = with(content.value.remoteData) { DownloadAssetMessageMetadata( content.value.name ?: "", diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCase.kt index 00a9df1a336..34655254acb 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCase.kt @@ -133,7 +133,12 @@ internal class ScheduleNewAssetMessageUseCaseImpl( FileSharingStatus.Value.Disabled -> return ScheduleNewAssetMessageResult.Failure.DisabledByTeam FileSharingStatus.Value.EnabledAll -> { /* no-op*/ } - is FileSharingStatus.Value.EnabledSome -> if (!validateAssetFileUseCase(assetName, it.state.allowedType)) { + is FileSharingStatus.Value.EnabledSome -> if (!validateAssetFileUseCase( + fileName = assetName, + mimeType = assetMimeType, + allowedExtension = it.state.allowedType + ) + ) { kaliumLogger.e("The asset message trying to be processed has invalid content data") return ScheduleNewAssetMessageResult.Failure.RestrictedFileType } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/ValidateAssetFileTypeUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/ValidateAssetFileTypeUseCase.kt index 6a15510cdc5..b4f887e1bb4 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/ValidateAssetFileTypeUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/ValidateAssetFileTypeUseCase.kt @@ -17,26 +17,95 @@ */ package com.wire.kalium.logic.feature.asset +import com.wire.kalium.logic.kaliumLogger + /** * Returns true if the file extension is present in file name and is allowed and false otherwise. * @param fileName the file name (with extension) to validate. * @param allowedExtension the list of allowed extension. */ interface ValidateAssetFileTypeUseCase { - operator fun invoke(fileName: String?, allowedExtension: List): Boolean + operator fun invoke( + fileName: String?, + mimeType: String, + allowedExtension: List + ): Boolean } internal class ValidateAssetFileTypeUseCaseImpl : ValidateAssetFileTypeUseCase { - override operator fun invoke(fileName: String?, allowedExtension: List): Boolean { - if (fileName == null) return false - - val split = fileName.split(".") - return if (split.size < 2) { - false + override operator fun invoke( + fileName: String?, + mimeType: String, + allowedExtension: List + ): Boolean { + kaliumLogger.d("Validating file type for $fileName with mimeType $mimeType is empty ${mimeType.isBlank()}") + val extension = if (fileName != null) { + extensionFromFileName(fileName) } else { - val allowedExtensionLowerCase = allowedExtension.map { it.lowercase() } - val extensions = split.subList(1, split.size).map { it.lowercase() } - extensions.all { it.isNotEmpty() && allowedExtensionLowerCase.contains(it) } + extensionFromMimeType(mimeType) } + return extension?.let { allowedExtension.contains(it) } ?: false + } + + private fun extensionFromFileName(fileName: String): String? = + fileName.substringAfterLast('.', "").takeIf { it.isNotEmpty() } + + private fun extensionFromMimeType(mimeType: String): String? = fileExtensions[mimeType] + + private companion object { + val fileExtensions = mapOf( + "video/3gpp" to "3gpp", + "audio/aac" to "aac", + "audio/amr" to "amr", + "video/x-msvideo" to "avi", + "image/bmp" to "bmp", + "text/css" to "css", + "text/csv" to "csv", + "application/msword" to "doc", + "application/vnd.openxmlformats-officedocument.wordprocessingml.document" to "docx", + "message/rfc822" to "eml", + "audio/flac" to "flac", + "image/gif" to "gif", + "text/html" to "html", + "image/vnd.microsoft.icon" to "ico", + "image/jpeg" to "jpeg", + "image/jpeg" to "jpg", + "image/jpeg" to "jfif", + "application/vnd.apple.keynote" to "key", + "audio/mp4" to "m4a", + "video/x-m4v" to "m4v", + "text/markdown" to "md", + "audio/midi" to "midi", + "video/x-matroska" to "mkv", + "video/quicktime" to "mov", + "audio/mpeg" to "mp3", + "video/mp4" to "mp4", + "video/mpeg" to "mpeg", + "application/vnd.ms-outlook" to "msg", + "application/vnd.oasis.opendocument.spreadsheet" to "ods", + "application/vnd.oasis.opendocument.text" to "odt", + "audio/ogg" to "ogg", + "application/pdf" to "pdf", + "image/jpeg" to "pjp", + "image/pjpeg" to "pjpeg", + "image/png" to "png", + "application/vnd.ms-powerpoint" to "ppt", + "application/vnd.openxmlformats-officedocument.presentationml.presentation" to "pptx", + "image/vnd.adobe.photoshop" to "psd", + "application/rtf" to "rtf", + "application/sql" to "sql", + "image/svg+xml" to "svg", + "application/x-tex" to "tex", + "image/tiff" to "tiff", + "text/plain" to "txt", + "text/x-vcard" to "vcf", + "audio/wav" to "wav", + "video/webm" to "webm", + "image/webp" to "webp", + "video/x-ms-wmv" to "wmv", + "application/vnd.ms-excel" to "xls", + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" to "xlsx", + "application/xml" to "xml" + ) } } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandler.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandler.kt index 9a71c5e2d13..b81bc3e0ded 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandler.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandler.kt @@ -24,7 +24,11 @@ import com.wire.kalium.logic.data.message.Message import com.wire.kalium.logic.data.message.MessageContent import com.wire.kalium.logic.data.message.MessageRepository import com.wire.kalium.logic.data.message.PersistMessageUseCase +<<<<<<< HEAD import com.wire.kalium.logic.data.message.hasValidData +======= +import com.wire.kalium.logic.data.message.getType +>>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) import com.wire.kalium.logic.feature.asset.ValidateAssetFileTypeUseCase import com.wire.kalium.logic.functional.onFailure import com.wire.kalium.logic.functional.onSuccess @@ -47,21 +51,53 @@ internal class AssetMessageHandlerImpl( kaliumLogger.e("The asset message trying to be processed has invalid content data") return } +<<<<<<< HEAD +======= + + val messageContent = message.content +>>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) userConfigRepository.isFileSharingEnabled().onSuccess { val isThisAssetAllowed = when (it.state) { - FileSharingStatus.Value.Disabled -> false - FileSharingStatus.Value.EnabledAll -> true + FileSharingStatus.Value.Disabled -> AssetRestrictionContinuationStrategy.Restrict + FileSharingStatus.Value.EnabledAll -> AssetRestrictionContinuationStrategy.Continue - is FileSharingStatus.Value.EnabledSome -> validateAssetMimeTypeUseCase( - messageContent.value.name, - it.state.allowedType - ) + is FileSharingStatus.Value.EnabledSome -> { + // If the asset message is missing the name, but it does have full + // asset data then we can not decide now if it is allowed or not + // it is safe to continue and the code later will check the original + // asset message and decide if it is allowed or not + if ( + message.content.value.name.isNullOrEmpty() && + message.content.value.isAssetDataComplete + ) { + kaliumLogger.e("The asset message trying to be processed has invalid data looking locally") + AssetRestrictionContinuationStrategy.RestrictIfThereIsNotOldMessageWithTheSameAssetID + } else { + validateAssetMimeTypeUseCase( + fileName = messageContent.value.name, + mimeType = messageContent.value.mimeType, + allowedExtension = it.state.allowedType + ).let { validateResult -> + if (validateResult) { + AssetRestrictionContinuationStrategy.Continue + } else { + AssetRestrictionContinuationStrategy.Restrict + } + } + } + } } - if (isThisAssetAllowed) { - processNonRestrictedAssetMessage(message, messageContent) - } else { - persistRestrictedAssetMessage(message, messageContent) + when (isThisAssetAllowed) { + AssetRestrictionContinuationStrategy.Continue -> processNonRestrictedAssetMessage(message, messageContent, false) + AssetRestrictionContinuationStrategy.RestrictIfThereIsNotOldMessageWithTheSameAssetID -> processNonRestrictedAssetMessage( + message, + messageContent, + true + ) + + AssetRestrictionContinuationStrategy.Restrict -> persistRestrictedAssetMessage(message, messageContent) + } } } @@ -77,23 +113,34 @@ internal class AssetMessageHandlerImpl( persistMessage(newMessage) } - private suspend fun processNonRestrictedAssetMessage(processedMessage: Message.Regular, assetContent: MessageContent.Asset) { + private suspend fun processNonRestrictedAssetMessage( + processedMessage: Message.Regular, + assetContent: MessageContent.Asset, + restrictIfNotAFollowUpMessage: Boolean + ) { messageRepository.getMessageById(processedMessage.conversationId, processedMessage.id).onFailure { // No asset message was received previously, so just persist the preview of the asset message // Web/Mac clients split the asset message delivery into 2. One with the preview metadata (assetName, assetSize...) and // with empty encryption keys and the second with empty metadata but all the correct encryption keys. We just want to // hide the preview of generic asset messages with empty encryption keys as a way to avoid user interaction with them. - val initialMessage = processedMessage.copy( - visibility = if (assetContent.value.shouldBeDisplayed) Message.Visibility.VISIBLE else Message.Visibility.HIDDEN - ) - persistMessage(initialMessage) + + if (restrictIfNotAFollowUpMessage) { + persistRestrictedAssetMessage(processedMessage, assetContent) + } else { + val initialMessage = processedMessage.copy( + visibility = if (assetContent.value.isAssetDataComplete) Message.Visibility.VISIBLE else Message.Visibility.HIDDEN + ) + persistMessage(initialMessage) + } }.onSuccess { persistedMessage -> val validDecryptionKeys = assetContent.value.remoteData // Check the second asset message is from the same original sender if (isSenderVerified(persistedMessage, processedMessage) && persistedMessage is Message.Regular) { // The second asset message received from Web/Mac clients contains the full asset decryption keys, so we need to update // the preview message persisted previously with the rest of the data - persistMessage(updateAssetMessageWithDecryptionKeys(persistedMessage, validDecryptionKeys)) + updateAssetMessageWithDecryptionKeys(persistedMessage, validDecryptionKeys)?.let { + persistMessage(it) + } } else { kaliumLogger.e("The previously persisted message has a different sender id than the one we are trying to process") } @@ -106,8 +153,21 @@ internal class AssetMessageHandlerImpl( private fun updateAssetMessageWithDecryptionKeys( persistedMessage: Message.Regular, remoteData: AssetContent.RemoteData - ): Message.Regular { - val assetMessageContent = persistedMessage.content as MessageContent.Asset + ): Message.Regular? { + val assetMessageContent = when (persistedMessage.content) { + is MessageContent.Asset -> persistedMessage.content + is MessageContent.RestrictedAsset -> { + // original message was a restricted asset message, ignoring + return null + } + + is MessageContent.FailedDecryption, + is MessageContent.Knock, + is MessageContent.Location, + is MessageContent.Composite, + is MessageContent.Text, + is MessageContent.Unknown -> error("Invalid asset message content type ${persistedMessage.content.getType()}") + } // The message was previously received with just metadata info, so let's update it with the raw data info return persistedMessage.copy( content = assetMessageContent.copy( @@ -120,3 +180,9 @@ internal class AssetMessageHandlerImpl( ) } } + +private sealed interface AssetRestrictionContinuationStrategy { + data object Continue : AssetRestrictionContinuationStrategy + data object Restrict : AssetRestrictionContinuationStrategy + data object RestrictIfThereIsNotOldMessageWithTheSameAssetID : AssetRestrictionContinuationStrategy +} diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCaseTest.kt index 7ba8fa79b65..5b4855da570 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCaseTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCaseTest.kt @@ -628,7 +628,13 @@ class ScheduleNewAssetMessageUseCaseTest { // Then assertTrue(result is ScheduleNewAssetMessageResult.Failure.RestrictedFileType) +<<<<<<< HEAD coVerify { arrangement.validateAssetMimeTypeUseCase(eq("some-asset.txt"), eq(listOf("png"))) } +======= + verify(arrangement.validateAssetMimeTypeUseCase) + .function(arrangement.validateAssetMimeTypeUseCase::invoke) + .with(eq("some-asset.txt"), eq("text/plain"), eq(listOf("png"))) +>>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) .wasInvoked(exactly = once) } @@ -667,7 +673,13 @@ class ScheduleNewAssetMessageUseCaseTest { // Then assertTrue(result is ScheduleNewAssetMessageResult.Success) +<<<<<<< HEAD coVerify { arrangement.validateAssetMimeTypeUseCase(eq("some-asset.png"), eq(listOf("png"))) } +======= + verify(arrangement.validateAssetMimeTypeUseCase) + .function(arrangement.validateAssetMimeTypeUseCase::invoke) + .with(eq("some-asset.png"), eq("image/png"), eq(listOf("png"))) +>>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) .wasInvoked(exactly = once) } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ValidateAssetFileTypeUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ValidateAssetFileTypeUseCaseTest.kt index 613f57bafe0..9d263a8e337 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ValidateAssetFileTypeUseCaseTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ValidateAssetFileTypeUseCaseTest.kt @@ -28,7 +28,7 @@ class ValidateAssetFileTypeUseCaseTest { fun givenRegularFileNameWithAllowedExtension_whenInvoke_thenBeApproved() = runTest { val (_, validate) = arrange {} - val result = validate("name.txt", listOf("txt", "jpg")) + val result = validate(fileName = "name.txt", mimeType = "", allowedExtension = listOf("txt", "jpg")) assertTrue(result) } @@ -37,7 +37,7 @@ class ValidateAssetFileTypeUseCaseTest { fun givenRegularFileNameWithNOTAllowedExtension_whenInvoke_thenBeRestricted() = runTest { val (_, validate) = arrange {} - val result = validate("name.php", listOf("txt", "jpg")) + val result = validate(fileName = "name.php", mimeType = "", allowedExtension = listOf("txt", "jpg")) assertFalse(result) } @@ -46,7 +46,7 @@ class ValidateAssetFileTypeUseCaseTest { fun givenRegularFileNameWithoutExtension_whenInvoke_thenBeRestricted() = runTest { val (_, validate) = arrange {} - val result = validate("name", listOf("txt", "jpg")) + val result = validate(fileName = "name", mimeType = "", allowedExtension = listOf("txt", "jpg")) assertFalse(result) } @@ -55,24 +55,40 @@ class ValidateAssetFileTypeUseCaseTest { fun givenNullFileName_whenInvoke_thenBeRestricted() = runTest { val (_, validate) = arrange {} - val result = validate(null, listOf("txt", "jpg")) + val result = validate(fileName = null, mimeType = "", allowedExtension = listOf("txt", "jpg")) assertFalse(result) } @Test - fun givenRegularFileNameWithFewExtensions_whenInvoke_thenEachExtensionIsChecked() = runTest { + fun givenFileNameIs() = runTest { val (_, validate) = arrange {} - val result1 = validate("name.php.txt", listOf("txt", "jpg")) - val result2 = validate("name.txt.php", listOf("txt", "jpg")) - val result3 = validate("name..txt.jpg", listOf("txt", "jpg")) - val result4 = validate("name.txt.php.txt.jpg", listOf("txt", "jpg")) + val result = validate(fileName = null, mimeType = "image/jpg", allowedExtension = listOf("txt", "jpg")) - assertFalse(result1) - assertFalse(result2) - assertFalse(result3) - assertFalse(result4) + assertFalse(result) + } + + @Test + fun givenNullFileNameAndValidMimeType_whenInvoke_thenMimeTypeIsChecked() = runTest { + val (_, validate) = arrange {} + + val result = validate(fileName = null, mimeType = "image/jpg", allowedExtension = listOf("txt", "jpg")) + + assertFalse(result) + } + + @Test + fun givenNullFileNameAndInvalidMimeType_whenInvoke_thenMimeTypeIsChecked() = runTest { + val (_, validate) = arrange {} + + val result = validate( + fileName = null, + mimeType = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + allowedExtension = listOf("txt", "jpg") + ) + + assertFalse(result) } private fun arrange(block: Arrangement.() -> Unit) = Arrangement(block).arrange() diff --git a/logic/src/jvmTest/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandlerTest.kt b/logic/src/jvmTest/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandlerTest.kt index 358d3cdb2a5..094638707f7 100644 --- a/logic/src/jvmTest/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandlerTest.kt +++ b/logic/src/jvmTest/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandlerTest.kt @@ -274,7 +274,18 @@ class AssetMessageHandlerTest { coVerify { arrangement.messageRepository.getMessageById(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) } .wasInvoked(exactly = once) +<<<<<<< HEAD coVerify { arrangement.validateAssetMimeType(eq(COMPLETE_ASSET_CONTENT.value.name), eq(isFileSharingEnabled.allowedType)) } +======= + verify(arrangement.messageRepository) + .suspendFunction(arrangement.messageRepository::getMessageById) + .with(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) + .wasInvoked(exactly = once) + + verify(arrangement.validateAssetMimeType) + .suspendFunction(arrangement.validateAssetMimeType::invoke) + .with(eq(COMPLETE_ASSET_CONTENT.value.name), eq("application/zip"), eq(isFileSharingEnabled.allowedType)) +>>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) .wasInvoked(exactly = once) } @@ -308,7 +319,18 @@ class AssetMessageHandlerTest { coVerify { arrangement.messageRepository.getMessageById(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) } .wasInvoked(exactly = once) +<<<<<<< HEAD coVerify { arrangement.validateAssetMimeType(eq(COMPLETE_ASSET_CONTENT.value.name), eq(isFileSharingEnabled.allowedType)) } +======= + verify(arrangement.messageRepository) + .suspendFunction(arrangement.messageRepository::getMessageById) + .with(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) + .wasInvoked(exactly = once) + + verify(arrangement.validateAssetMimeType) + .suspendFunction(arrangement.validateAssetMimeType::invoke) + .with(eq(COMPLETE_ASSET_CONTENT.value.name), eq("application/zip"), eq(isFileSharingEnabled.allowedType)) +>>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) .wasInvoked(exactly = once) } @@ -346,6 +368,103 @@ class AssetMessageHandlerTest { .wasNotInvoked() } + @Test + fun givenFileWithNullNameAndCompleteData_whenProcessingCheckAPreviousAssetWithTheSameIDIsRestricted_thenDoNotStore() = runTest { + // Given + val messageCOntant = MessageContent.Asset( + AssetContent( + sizeInBytes = 100, + name = null, + mimeType = "", + metadata = null, + remoteData = AssetContent.RemoteData( + otrKey = "otrKey".toByteArray(), + sha256 = "sha256".toByteArray(), + assetId = "some-asset-id", + assetDomain = "some-asset-domain", + assetToken = "some-asset-token", + encryptionAlgorithm = MessageEncryptionAlgorithm.AES_GCM + ), + uploadStatus = Message.UploadStatus.NOT_UPLOADED, + downloadStatus = Message.DownloadStatus.NOT_DOWNLOADED + ) + + ) + val assetMessage = COMPLETE_ASSET_MESSAGE.copy(content = messageCOntant) + + val previewAssetMessage = PREVIEW_ASSET_MESSAGE.copy( + visibility = Message.Visibility.HIDDEN, + content = MessageContent.RestrictedAsset("application/zip", 500, "some-asset-name.zip.") + ) + + val isFileSharingEnabled = FileSharingStatus.Value.EnabledSome(listOf("txt", "png", "zip")) + val (arrangement, assetMessageHandler) = Arrangement() + .withSuccessfulFileSharingFlag(isFileSharingEnabled) + .withSuccessfulStoredMessage(previewAssetMessage) + .arrange() + + // When + assetMessageHandler.handle(assetMessage) + + // Then + verify(arrangement.persistMessage) + .suspendFunction(arrangement.persistMessage::invoke) + .with(any()) + .wasNotInvoked() + + verify(arrangement.messageRepository) + .suspendFunction(arrangement.messageRepository::getMessageById) + .with(eq(assetMessage.conversationId), eq(assetMessage.id)) + .wasInvoked(exactly = once) + } + + @Test + fun givenFileWithNullNameAndCompleteData_whenProcessingCheckAPreviousAssetWithTheSameIDIsMissing_thenStoreAsRestricted() = runTest { + // Given + val messageCOntant = MessageContent.Asset( + AssetContent( + sizeInBytes = 100, + name = null, + mimeType = "", + metadata = null, + remoteData = AssetContent.RemoteData( + otrKey = "otrKey".toByteArray(), + sha256 = "sha256".toByteArray(), + assetId = "some-asset-id", + assetDomain = "some-asset-domain", + assetToken = "some-asset-token", + encryptionAlgorithm = MessageEncryptionAlgorithm.AES_GCM + ), + uploadStatus = Message.UploadStatus.NOT_UPLOADED, + downloadStatus = Message.DownloadStatus.NOT_DOWNLOADED + ) + + ) + val assetMessage = COMPLETE_ASSET_MESSAGE.copy(content = messageCOntant) + + val storedMessage = assetMessage.copy(content = MessageContent.RestrictedAsset(mimeType = "", sizeInBytes = 100, name = "")) + val isFileSharingEnabled = FileSharingStatus.Value.EnabledSome(listOf("txt", "png", "zip")) + val (arrangement, assetMessageHandler) = Arrangement() + .withSuccessfulFileSharingFlag(isFileSharingEnabled) + .withSuccessfulStoredMessage(null) + .withSuccessfulPersistMessageUseCase(storedMessage) + .arrange() + + // When + assetMessageHandler.handle(assetMessage) + + // Then + verify(arrangement.persistMessage) + .suspendFunction(arrangement.persistMessage::invoke) + .with(any()) + .wasInvoked(exactly = once) + + verify(arrangement.messageRepository) + .suspendFunction(arrangement.messageRepository::getMessageById) + .with(eq(assetMessage.conversationId), eq(assetMessage.id)) + .wasInvoked(exactly = once) + } + private class Arrangement { @Mock From 4813237bb1a004450a60fe8d1eb8f0efd46a6b0d Mon Sep 17 00:00:00 2001 From: Mohamad Jaara Date: Fri, 13 Sep 2024 18:32:13 +0200 Subject: [PATCH 2/2] merge conflicts --- .../feature/asset/GetMessageAssetUseCase.kt | 9 -- .../receiver/asset/AssetMessageHandler.kt | 20 ++--- .../ScheduleNewAssetMessageUseCaseTest.kt | 30 +++---- .../receiver/asset/AssetMessageHandlerTest.kt | 84 ++++++++----------- 4 files changed, 58 insertions(+), 85 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCase.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCase.kt index 4ba441f7d45..08639c7cfb2 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCase.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/asset/GetMessageAssetUseCase.kt @@ -79,16 +79,7 @@ internal class GetMessageAssetUseCaseImpl( }, { message -> when (val content = message.content) { is MessageContent.Asset -> { -<<<<<<< HEAD // TODO isIncompleteImage should be used here for incomplete messages -======= - val assetDownloadStatus = content.value.downloadStatus - val assetUploadStatus = content.value.uploadStatus - val wasDownloaded: Boolean = assetDownloadStatus == SAVED_INTERNALLY || assetDownloadStatus == SAVED_EXTERNALLY - // assets uploaded by other clients have upload status NOT_UPLOADED - val alreadyUploaded: Boolean = (assetUploadStatus == NOT_UPLOADED && content.value.isAssetDataComplete) - || assetUploadStatus == UPLOADED ->>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) val assetMetadata = with(content.value.remoteData) { DownloadAssetMessageMetadata( content.value.name ?: "", diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandler.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandler.kt index b81bc3e0ded..3bc9fa7ac2e 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandler.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandler.kt @@ -24,15 +24,12 @@ import com.wire.kalium.logic.data.message.Message import com.wire.kalium.logic.data.message.MessageContent import com.wire.kalium.logic.data.message.MessageRepository import com.wire.kalium.logic.data.message.PersistMessageUseCase -<<<<<<< HEAD -import com.wire.kalium.logic.data.message.hasValidData -======= import com.wire.kalium.logic.data.message.getType ->>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) import com.wire.kalium.logic.feature.asset.ValidateAssetFileTypeUseCase import com.wire.kalium.logic.functional.onFailure import com.wire.kalium.logic.functional.onSuccess import com.wire.kalium.logic.kaliumLogger +import com.wire.kalium.logic.sync.receiver.conversation.message.hasValidData internal interface AssetMessageHandler { suspend fun handle(message: Message.Regular) @@ -46,16 +43,13 @@ internal class AssetMessageHandlerImpl( ) : AssetMessageHandler { override suspend fun handle(message: Message.Regular) { - val messageContent = message.content - if (messageContent !is MessageContent.Asset) { + if (message.content !is MessageContent.Asset) { kaliumLogger.e("The asset message trying to be processed has invalid content data") return } -<<<<<<< HEAD -======= - val messageContent = message.content ->>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) + val messageContent = message.content as MessageContent.Asset + userConfigRepository.isFileSharingEnabled().onSuccess { val isThisAssetAllowed = when (it.state) { FileSharingStatus.Value.Disabled -> AssetRestrictionContinuationStrategy.Restrict @@ -67,8 +61,8 @@ internal class AssetMessageHandlerImpl( // it is safe to continue and the code later will check the original // asset message and decide if it is allowed or not if ( - message.content.value.name.isNullOrEmpty() && - message.content.value.isAssetDataComplete + messageContent.value.name.isNullOrEmpty() && + messageContent.value.isAssetDataComplete ) { kaliumLogger.e("The asset message trying to be processed has invalid data looking locally") AssetRestrictionContinuationStrategy.RestrictIfThereIsNotOldMessageWithTheSameAssetID @@ -155,7 +149,7 @@ internal class AssetMessageHandlerImpl( remoteData: AssetContent.RemoteData ): Message.Regular? { val assetMessageContent = when (persistedMessage.content) { - is MessageContent.Asset -> persistedMessage.content + is MessageContent.Asset -> persistedMessage.content as MessageContent.Asset is MessageContent.RestrictedAsset -> { // original message was a restricted asset message, ignoring return null diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCaseTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCaseTest.kt index 5b4855da570..52de2380446 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCaseTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/feature/asset/ScheduleNewAssetMessageUseCaseTest.kt @@ -628,13 +628,13 @@ class ScheduleNewAssetMessageUseCaseTest { // Then assertTrue(result is ScheduleNewAssetMessageResult.Failure.RestrictedFileType) -<<<<<<< HEAD - coVerify { arrangement.validateAssetMimeTypeUseCase(eq("some-asset.txt"), eq(listOf("png"))) } -======= - verify(arrangement.validateAssetMimeTypeUseCase) - .function(arrangement.validateAssetMimeTypeUseCase::invoke) - .with(eq("some-asset.txt"), eq("text/plain"), eq(listOf("png"))) ->>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) + coVerify { + arrangement.validateAssetMimeTypeUseCase( + fileName = eq("some-asset.txt"), + mimeType = eq("text/plain"), + allowedExtension = eq(listOf("png")) + ) + } .wasInvoked(exactly = once) } @@ -673,13 +673,13 @@ class ScheduleNewAssetMessageUseCaseTest { // Then assertTrue(result is ScheduleNewAssetMessageResult.Success) -<<<<<<< HEAD - coVerify { arrangement.validateAssetMimeTypeUseCase(eq("some-asset.png"), eq(listOf("png"))) } -======= - verify(arrangement.validateAssetMimeTypeUseCase) - .function(arrangement.validateAssetMimeTypeUseCase::invoke) - .with(eq("some-asset.png"), eq("image/png"), eq(listOf("png"))) ->>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) + coVerify { + arrangement.validateAssetMimeTypeUseCase( + fileName = eq("some-asset.png"), + mimeType = eq("image/png"), + allowedExtension = eq(listOf("png")) + ) + } .wasInvoked(exactly = once) } @@ -733,7 +733,7 @@ class ScheduleNewAssetMessageUseCaseTest { fun withValidateAsseMimeTypeResult(result: Boolean) = apply { every { - validateAssetMimeTypeUseCase.invoke(any(), any()) + validateAssetMimeTypeUseCase.invoke(any(), any(), any()) }.returns(result) } diff --git a/logic/src/jvmTest/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandlerTest.kt b/logic/src/jvmTest/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandlerTest.kt index 094638707f7..64b4db40220 100644 --- a/logic/src/jvmTest/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandlerTest.kt +++ b/logic/src/jvmTest/kotlin/com/wire/kalium/logic/sync/receiver/asset/AssetMessageHandlerTest.kt @@ -271,22 +271,21 @@ class AssetMessageHandlerTest { }) }.wasInvoked(exactly = once) - coVerify { arrangement.messageRepository.getMessageById(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) } - .wasInvoked(exactly = once) - -<<<<<<< HEAD - coVerify { arrangement.validateAssetMimeType(eq(COMPLETE_ASSET_CONTENT.value.name), eq(isFileSharingEnabled.allowedType)) } -======= - verify(arrangement.messageRepository) - .suspendFunction(arrangement.messageRepository::getMessageById) - .with(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) + coVerify { + arrangement.messageRepository.getMessageById( + eq(previewAssetMessage.conversationId), + eq(previewAssetMessage.id) + ) + } .wasInvoked(exactly = once) - verify(arrangement.validateAssetMimeType) - .suspendFunction(arrangement.validateAssetMimeType::invoke) - .with(eq(COMPLETE_ASSET_CONTENT.value.name), eq("application/zip"), eq(isFileSharingEnabled.allowedType)) ->>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) - .wasInvoked(exactly = once) + coVerify { + arrangement.validateAssetMimeType( + fileName = eq(COMPLETE_ASSET_CONTENT.value.name), + mimeType = eq("application/zip"), + allowedExtension = eq(isFileSharingEnabled.allowedType) + ) + }.wasInvoked(exactly = once) } @Test @@ -316,22 +315,20 @@ class AssetMessageHandlerTest { }) }.wasInvoked(exactly = once) - coVerify { arrangement.messageRepository.getMessageById(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) } - .wasInvoked(exactly = once) - -<<<<<<< HEAD - coVerify { arrangement.validateAssetMimeType(eq(COMPLETE_ASSET_CONTENT.value.name), eq(isFileSharingEnabled.allowedType)) } -======= - verify(arrangement.messageRepository) - .suspendFunction(arrangement.messageRepository::getMessageById) - .with(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) - .wasInvoked(exactly = once) + coVerify { + arrangement.messageRepository.getMessageById( + conversationId = eq(previewAssetMessage.conversationId), + messageUuid = eq(previewAssetMessage.id) + ) + }.wasInvoked(exactly = once) - verify(arrangement.validateAssetMimeType) - .suspendFunction(arrangement.validateAssetMimeType::invoke) - .with(eq(COMPLETE_ASSET_CONTENT.value.name), eq("application/zip"), eq(isFileSharingEnabled.allowedType)) ->>>>>>> 538bae1769 (fix: handle the case where asset name can be missing (#2995)) - .wasInvoked(exactly = once) + coVerify { + arrangement.validateAssetMimeType( + fileName = eq(COMPLETE_ASSET_CONTENT.value.name), + mimeType = eq("application/zip"), + allowedExtension = eq(isFileSharingEnabled.allowedType) + ) + }.wasInvoked(exactly = once) } @Test @@ -364,7 +361,7 @@ class AssetMessageHandlerTest { coVerify { arrangement.messageRepository.getMessageById(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) } .wasNotInvoked() - coVerify { arrangement.validateAssetMimeType(any(), any>()) } + coVerify { arrangement.validateAssetMimeType(any(), any(), any>()) } .wasNotInvoked() } @@ -385,8 +382,6 @@ class AssetMessageHandlerTest { assetToken = "some-asset-token", encryptionAlgorithm = MessageEncryptionAlgorithm.AES_GCM ), - uploadStatus = Message.UploadStatus.NOT_UPLOADED, - downloadStatus = Message.DownloadStatus.NOT_DOWNLOADED ) ) @@ -407,15 +402,14 @@ class AssetMessageHandlerTest { assetMessageHandler.handle(assetMessage) // Then - verify(arrangement.persistMessage) - .suspendFunction(arrangement.persistMessage::invoke) - .with(any()) + coVerify { arrangement.persistMessage(any()) } .wasNotInvoked() - verify(arrangement.messageRepository) - .suspendFunction(arrangement.messageRepository::getMessageById) - .with(eq(assetMessage.conversationId), eq(assetMessage.id)) - .wasInvoked(exactly = once) + coVerify { + arrangement.messageRepository.getMessageById( + eq(assetMessage.conversationId), eq(assetMessage.id) + ) + }.wasInvoked(exactly = once) } @Test @@ -435,8 +429,6 @@ class AssetMessageHandlerTest { assetToken = "some-asset-token", encryptionAlgorithm = MessageEncryptionAlgorithm.AES_GCM ), - uploadStatus = Message.UploadStatus.NOT_UPLOADED, - downloadStatus = Message.DownloadStatus.NOT_DOWNLOADED ) ) @@ -454,14 +446,10 @@ class AssetMessageHandlerTest { assetMessageHandler.handle(assetMessage) // Then - verify(arrangement.persistMessage) - .suspendFunction(arrangement.persistMessage::invoke) - .with(any()) + coVerify { arrangement.persistMessage(any()) } .wasInvoked(exactly = once) - verify(arrangement.messageRepository) - .suspendFunction(arrangement.messageRepository::getMessageById) - .with(eq(assetMessage.conversationId), eq(assetMessage.id)) + coVerify { arrangement.messageRepository.getMessageById(eq(assetMessage.conversationId), eq(assetMessage.id)) } .wasInvoked(exactly = once) } @@ -484,7 +472,7 @@ class AssetMessageHandlerTest { fun withValidateAssetMime(result: Boolean) = apply { every { - validateAssetMimeType.invoke(any(), any()) + validateAssetMimeType.invoke(any(), any(), any()) }.returns(result) }