From 222b1b6ce29925a8be385003cd9840d7d148955d Mon Sep 17 00:00:00 2001 From: Mohamad Jaara Date: Fri, 13 Sep 2024 13:08:02 +0200 Subject: [PATCH 1/2] fix: images form iOS are blocked when restrictions are applied (#3006) --- .../receiver/asset/AssetMessageHandler.kt | 23 +++++-------- .../receiver/asset/AssetMessageHandlerTest.kt | 34 ++++++++++++++++--- 2 files changed, 39 insertions(+), 18 deletions(-) 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 3bc9fa7ac2..b99adccea0 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 @@ -60,23 +60,18 @@ internal class AssetMessageHandlerImpl( // 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 ( - messageContent.value.name.isNullOrEmpty() && - messageContent.value.isAssetDataComplete - ) { - kaliumLogger.e("The asset message trying to be processed has invalid data looking locally") - AssetRestrictionContinuationStrategy.RestrictIfThereIsNotOldMessageWithTheSameAssetID - } else { - validateAssetMimeTypeUseCase( + if (validateAssetMimeTypeUseCase( fileName = messageContent.value.name, mimeType = messageContent.value.mimeType, allowedExtension = it.state.allowedType - ).let { validateResult -> - if (validateResult) { - AssetRestrictionContinuationStrategy.Continue - } else { - AssetRestrictionContinuationStrategy.Restrict - } + ) + ) { + AssetRestrictionContinuationStrategy.Continue + } else { + if (messageContent.value.name.isNullOrEmpty() && messageContent.value.isAssetDataComplete) { + AssetRestrictionContinuationStrategy.RestrictIfThereIsNotOldMessageWithTheSameAssetID + } else { + AssetRestrictionContinuationStrategy.Restrict } } } 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 64b4db4022..6fcdbc175c 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 @@ -36,6 +36,8 @@ import com.wire.kalium.logic.functional.Either import com.wire.kalium.util.time.UNIX_FIRST_DATE import io.mockative.Mock import io.mockative.any +import io.mockative.anything +import io.mockative.classOf import io.mockative.coEvery import io.mockative.coVerify import io.mockative.eq @@ -251,7 +253,7 @@ class AssetMessageHandlerTest { val isFileSharingEnabled = FileSharingStatus.Value.EnabledSome(listOf("txt", "png", "zip")) val (arrangement, assetMessageHandler) = Arrangement() .withSuccessfulFileSharingFlag(isFileSharingEnabled) - .withValidateAssetMime(true) + .withValidateAssetFileType(true) .withSuccessfulStoredMessage(previewAssetMessage) .withSuccessfulPersistMessageUseCase(updateAssetMessage) .arrange() @@ -279,6 +281,15 @@ class AssetMessageHandlerTest { } .wasInvoked(exactly = once) + verify(arrangement.messageRepository) + .suspendFunction(arrangement.messageRepository::getMessageById) + .with(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) + .wasInvoked(exactly = once) + + verify(arrangement.validateAssetFileTypeUseCase) + .suspendFunction(arrangement.validateAssetFileTypeUseCase::invoke) + .with(eq(COMPLETE_ASSET_CONTENT.value.name), eq("application/zip"), eq(isFileSharingEnabled.allowedType)) + .wasInvoked(exactly = once) coVerify { arrangement.validateAssetMimeType( fileName = eq(COMPLETE_ASSET_CONTENT.value.name), @@ -296,7 +307,7 @@ class AssetMessageHandlerTest { val isFileSharingEnabled = FileSharingStatus.Value.EnabledSome(listOf("txt", "png")) val (arrangement, assetMessageHandler) = Arrangement() .withSuccessfulFileSharingFlag(isFileSharingEnabled) - .withValidateAssetMime(true) + .withValidateAssetFileType(true) .withSuccessfulStoredMessage(previewAssetMessage) .withSuccessfulPersistMessageUseCase(updateAssetMessage) .arrange() @@ -329,6 +340,10 @@ class AssetMessageHandlerTest { allowedExtension = eq(isFileSharingEnabled.allowedType) ) }.wasInvoked(exactly = once) + verify(arrangement.validateAssetFileTypeUseCase) + .suspendFunction(arrangement.validateAssetFileTypeUseCase::invoke) + .with(eq(COMPLETE_ASSET_CONTENT.value.name), eq("application/zip"), eq(isFileSharingEnabled.allowedType)) + .wasInvoked(exactly = once) } @Test @@ -339,7 +354,7 @@ class AssetMessageHandlerTest { val isFileSharingEnabled = FileSharingStatus.Value.Disabled val (arrangement, assetMessageHandler) = Arrangement() .withSuccessfulFileSharingFlag(isFileSharingEnabled) - .withValidateAssetMime(true) + .withValidateAssetFileType(true) .withSuccessfulStoredMessage(previewAssetMessage) .withSuccessfulPersistMessageUseCase(updateAssetMessage) .arrange() @@ -362,6 +377,9 @@ class AssetMessageHandlerTest { .wasNotInvoked() coVerify { arrangement.validateAssetMimeType(any(), any(), any>()) } + verify(arrangement.validateAssetFileTypeUseCase) + .suspendFunction(arrangement.validateAssetFileTypeUseCase::invoke) + .with(any(), any>()) .wasNotInvoked() } @@ -396,6 +414,7 @@ class AssetMessageHandlerTest { val (arrangement, assetMessageHandler) = Arrangement() .withSuccessfulFileSharingFlag(isFileSharingEnabled) .withSuccessfulStoredMessage(previewAssetMessage) + .withValidateAssetFileType(true) .arrange() // When @@ -440,6 +459,7 @@ class AssetMessageHandlerTest { .withSuccessfulFileSharingFlag(isFileSharingEnabled) .withSuccessfulStoredMessage(null) .withSuccessfulPersistMessageUseCase(storedMessage) + .withValidateAssetFileType(true) .arrange() // When @@ -466,14 +486,20 @@ class AssetMessageHandlerTest { @Mock val validateAssetMimeType = mock(ValidateAssetFileTypeUseCase::class) + val validateAssetFileTypeUseCase = mock(classOf()) private val assetMessageHandlerImpl = - AssetMessageHandlerImpl(messageRepository, persistMessage, userConfigRepository, validateAssetMimeType) + AssetMessageHandlerImpl(messageRepository, persistMessage, userConfigRepository, validateAssetFileTypeUseCase) fun withValidateAssetMime(result: Boolean) = apply { every { validateAssetMimeType.invoke(any(), any(), any()) }.returns(result) + fun withValidateAssetFileType(result: Boolean) = apply { + given(validateAssetFileTypeUseCase) + .function(validateAssetFileTypeUseCase::invoke) + .whenInvokedWith(anything(), any(), any()) + .thenReturn(result) } fun withSuccessfulFileSharingFlag(value: FileSharingStatus.Value) = apply { From f1682d5473e4a2ba4c8d9a95ff3373f11f337d91 Mon Sep 17 00:00:00 2001 From: Mohamad Jaara Date: Sun, 15 Sep 2024 18:48:27 +0200 Subject: [PATCH 2/2] cherry pick and resolve conflicts --- .../receiver/asset/AssetMessageHandlerTest.kt | 37 +++---------------- 1 file changed, 6 insertions(+), 31 deletions(-) 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 6fcdbc175c..fae069d261 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 @@ -36,7 +36,6 @@ import com.wire.kalium.logic.functional.Either import com.wire.kalium.util.time.UNIX_FIRST_DATE import io.mockative.Mock import io.mockative.any -import io.mockative.anything import io.mockative.classOf import io.mockative.coEvery import io.mockative.coVerify @@ -278,20 +277,10 @@ class AssetMessageHandlerTest { eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id) ) - } - .wasInvoked(exactly = once) - - verify(arrangement.messageRepository) - .suspendFunction(arrangement.messageRepository::getMessageById) - .with(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) - .wasInvoked(exactly = once) + }.wasInvoked(exactly = once) - verify(arrangement.validateAssetFileTypeUseCase) - .suspendFunction(arrangement.validateAssetFileTypeUseCase::invoke) - .with(eq(COMPLETE_ASSET_CONTENT.value.name), eq("application/zip"), eq(isFileSharingEnabled.allowedType)) - .wasInvoked(exactly = once) coVerify { - arrangement.validateAssetMimeType( + arrangement.validateAssetFileTypeUseCase( fileName = eq(COMPLETE_ASSET_CONTENT.value.name), mimeType = eq("application/zip"), allowedExtension = eq(isFileSharingEnabled.allowedType) @@ -334,16 +323,12 @@ class AssetMessageHandlerTest { }.wasInvoked(exactly = once) coVerify { - arrangement.validateAssetMimeType( + arrangement.validateAssetFileTypeUseCase( fileName = eq(COMPLETE_ASSET_CONTENT.value.name), mimeType = eq("application/zip"), allowedExtension = eq(isFileSharingEnabled.allowedType) ) }.wasInvoked(exactly = once) - verify(arrangement.validateAssetFileTypeUseCase) - .suspendFunction(arrangement.validateAssetFileTypeUseCase::invoke) - .with(eq(COMPLETE_ASSET_CONTENT.value.name), eq("application/zip"), eq(isFileSharingEnabled.allowedType)) - .wasInvoked(exactly = once) } @Test @@ -376,11 +361,7 @@ class AssetMessageHandlerTest { coVerify { arrangement.messageRepository.getMessageById(eq(previewAssetMessage.conversationId), eq(previewAssetMessage.id)) } .wasNotInvoked() - coVerify { arrangement.validateAssetMimeType(any(), any(), any>()) } - verify(arrangement.validateAssetFileTypeUseCase) - .suspendFunction(arrangement.validateAssetFileTypeUseCase::invoke) - .with(any(), any>()) - .wasNotInvoked() + coVerify { arrangement.validateAssetFileTypeUseCase(any(), any(), any>()) } } @Test @@ -485,21 +466,15 @@ class AssetMessageHandlerTest { val userConfigRepository = mock(UserConfigRepository::class) @Mock - val validateAssetMimeType = mock(ValidateAssetFileTypeUseCase::class) val validateAssetFileTypeUseCase = mock(classOf()) private val assetMessageHandlerImpl = AssetMessageHandlerImpl(messageRepository, persistMessage, userConfigRepository, validateAssetFileTypeUseCase) - fun withValidateAssetMime(result: Boolean) = apply { + fun withValidateAssetFileType(result: Boolean) = apply { every { - validateAssetMimeType.invoke(any(), any(), any()) + validateAssetFileTypeUseCase.invoke(any(), any(), any()) }.returns(result) - fun withValidateAssetFileType(result: Boolean) = apply { - given(validateAssetFileTypeUseCase) - .function(validateAssetFileTypeUseCase::invoke) - .whenInvokedWith(anything(), any(), any()) - .thenReturn(result) } fun withSuccessfulFileSharingFlag(value: FileSharingStatus.Value) = apply {