From f7670816940f97e7b1c4c947f8c2af17f3bebd96 Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Fri, 10 Jan 2025 14:43:44 +0200 Subject: [PATCH 1/7] Use allow and block lists for uploading files and images in the attachment uploader and in image and file pickers --- Sources/StreamChat/ChatClient.swift | 4 + .../Database/DTOs/AttachmentDTO.swift | 6 + Sources/StreamChat/Models/AppSettings.swift | 69 +++++++++ .../Background/AttachmentQueueUploader.swift | 101 +++++++++---- .../StreamChatUI/Composer/ComposerVC.swift | 21 ++- StreamChat.xcodeproj/project.pbxproj | 4 + .../Models/AppSettings_Tests.swift | 139 ++++++++++++++++++ .../AttachmentQueueUploader_Tests.swift | 117 +++++++++++++++ 8 files changed, 427 insertions(+), 34 deletions(-) create mode 100644 Tests/StreamChatTests/Models/AppSettings_Tests.swift diff --git a/Sources/StreamChat/ChatClient.swift b/Sources/StreamChat/ChatClient.swift index 3627ed00da..3009deca4d 100644 --- a/Sources/StreamChat/ChatClient.swift +++ b/Sources/StreamChat/ChatClient.swift @@ -578,6 +578,8 @@ public class ChatClient { case let .success(payload): let appSettings = payload.asModel() self?.appSettings = appSettings + try? self?.backgroundWorker(of: AttachmentQueueUploader.self) + .setAppSettings(appSettings) completion?(.success(appSettings)) case let .failure(error): completion?(.failure(error)) @@ -614,6 +616,8 @@ public class ChatClient { attachmentPostProcessor: config.uploadedAttachmentPostProcessor ) ] + try? backgroundWorker(of: AttachmentQueueUploader.self) + .setAppSettings(appSettings) } func completeConnectionIdWaiters(connectionId: String?) { diff --git a/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift b/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift index e1e832e698..52dcfbb679 100644 --- a/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift +++ b/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift @@ -375,6 +375,12 @@ extension ClientError { super.init("There is no `AttachmentDTO` instance in the DB matching id: \(id).") } } + + final class AttachmentTypeDisallowed: ClientError { + init(id: AttachmentId, attachmentType: AttachmentType) { + super.init("`AttachmentDTO` with \(id) has type \(attachmentType) which is marked as disallowed on the Stream dashboard.") + } + } final class AttachmentEditing: ClientError { init(id: AttachmentId, reason: String) { diff --git a/Sources/StreamChat/Models/AppSettings.swift b/Sources/StreamChat/Models/AppSettings.swift index fb040c2469..a997c465ab 100644 --- a/Sources/StreamChat/Models/AppSettings.swift +++ b/Sources/StreamChat/Models/AppSettings.swift @@ -3,6 +3,7 @@ // import Foundation +import MobileCoreServices /// A type representing the app settings. public struct AppSettings { @@ -57,3 +58,71 @@ extension AppSettingsPayload.UploadConfigPayload { ) } } + +// MARK: - Validation + +extension AppSettings.UploadConfig { + // MARK: - UTI Validation + + public var allowedUTITypes: [String] { + allowedMimeTypes.compactMap { $0.utiType(mime: true) } + + allowedFileExtensions.compactMap { $0.utiType(mime: false) } + } + + public var blockedUTITypes: [String] { + blockedMimeTypes.compactMap { $0.utiType(mime: true) } + + blockedFileExtensions.compactMap { $0.utiType(mime: false) } + } + + // MARK: - URL Validation + + func isAllowed(localURL: URL) -> Bool { + guard !localURL.pathExtension.isEmpty else { return true } + + if !allowedFileExtensions.isEmpty || !blockedFileExtensions.isEmpty { + if !isAllowed(pathExtension: localURL.pathExtension.lowercased()) { + return false + } + } + if !allowedMimeTypes.isEmpty || !blockedMimeTypes.isEmpty { + let mimeType = AttachmentFileType(ext: localURL.pathExtension).mimeType.lowercased() + if !isAllowed(mimeType: mimeType) { + return false + } + } + return true + } + + private func isAllowed(pathExtension: String) -> Bool { + let isBlocked = blockedFileExtensions.contains { blocked in + blocked.drop(while: { $0 == Character(".") }).caseInsensitiveCompare(pathExtension) == .orderedSame + } + guard !isBlocked else { return false } + guard !allowedFileExtensions.isEmpty else { return true } + return allowedFileExtensions.contains { allowed in + allowed.drop(while: { $0 == Character(".") }).caseInsensitiveCompare(pathExtension) == .orderedSame + } + } + + private func isAllowed(mimeType: String) -> Bool { + let isBlocked = blockedMimeTypes.contains { blocked in + blocked.caseInsensitiveCompare(mimeType) == .orderedSame + } + guard !isBlocked else { return false } + guard !allowedMimeTypes.isEmpty else { return true } + return allowedMimeTypes.contains { allowed in + allowed.caseInsensitiveCompare(mimeType) == .orderedSame + } + } +} + +private extension String { + func utiType(mime: Bool) -> String? { + let string = mime ? self : String(drop(while: { $0 == Character(".") })) + return UTTypeCreatePreferredIdentifierForTag( + mime ? kUTTagClassMIMEType : kUTTagClassFilenameExtension, + string as CFString, + nil + )?.takeRetainedValue() as? String + } +} diff --git a/Sources/StreamChat/Workers/Background/AttachmentQueueUploader.swift b/Sources/StreamChat/Workers/Background/AttachmentQueueUploader.swift index f1faa2033b..7b1b7ab898 100644 --- a/Sources/StreamChat/Workers/Background/AttachmentQueueUploader.swift +++ b/Sources/StreamChat/Workers/Background/AttachmentQueueUploader.swift @@ -27,7 +27,9 @@ class AttachmentQueueUploader: Worker { private let attachmentUpdater = AnyAttachmentUpdater() private let attachmentStorage = AttachmentStorage() private var continuations = [AttachmentId: CheckedContinuation]() - private let continuationsQueue = DispatchQueue(label: "co.getStream.ChatClient.AttachmentQueueUploader") + private let queue = DispatchQueue(label: "co.getStream.ChatClient.AttachmentQueueUploader", target: .global(qos: .utility)) + private var fileUploadConfig: AppSettings.UploadConfig? + private var imageUploadConfig: AppSettings.UploadConfig? var minSignificantUploadingProgressChange: Double = 0.05 @@ -43,6 +45,13 @@ class AttachmentQueueUploader: Worker { startObserving() } + + func setAppSettings(_ appSettings: AppSettings?) { + queue.async { [weak self] in + self?.fileUploadConfig = appSettings?.fileUploadConfig + self?.imageUploadConfig = appSettings?.imageUploadConfig + } + } // MARK: - Private @@ -76,45 +85,59 @@ class AttachmentQueueUploader: Worker { } private func uploadAttachment(with id: AttachmentId) { - prepareAttachmentForUpload(with: id) { [weak self] attachment in - guard let attachment = attachment else { - self?.removePendingAttachment(with: id, result: .failure(ClientError.AttachmentDoesNotExist(id: id))) - return - } - - self?.apiClient.uploadAttachment( - attachment, - progress: { + prepareAttachmentForUpload(with: id) { [weak self] result in + switch result { + case .failure(let error): + if error is ClientError.AttachmentDoesNotExist { + self?.removePendingAttachment(with: id, result: .failure(error)) + } else { self?.updateAttachmentIfNeeded( attachmentId: id, uploadedAttachment: nil, - newState: .uploading(progress: $0), - completion: {} - ) - }, - completion: { result in - self?.updateAttachmentIfNeeded( - attachmentId: id, - uploadedAttachment: result.value, - newState: result.error == nil ? .uploaded : .uploadingFailed, + newState: .uploadingFailed, completion: { - self?.removePendingAttachment(with: id, result: result) + self?.removePendingAttachment(with: id, result: .failure(error)) } ) } - ) + case .success(let attachment): + self?.apiClient.uploadAttachment( + attachment, + progress: { + self?.updateAttachmentIfNeeded( + attachmentId: id, + uploadedAttachment: nil, + newState: .uploading(progress: $0), + completion: {} + ) + }, + completion: { result in + self?.updateAttachmentIfNeeded( + attachmentId: id, + uploadedAttachment: result.value, + newState: result.error == nil ? .uploaded : .uploadingFailed, + completion: { + self?.removePendingAttachment(with: id, result: result) + } + ) + } + ) + } } } - private func prepareAttachmentForUpload(with id: AttachmentId, completion: @escaping (AnyChatMessageAttachment?) -> Void) { + private func prepareAttachmentForUpload(with id: AttachmentId, completion: @escaping (Result) -> Void) { let attachmentStorage = self.attachmentStorage - database.write { session in + var model: AnyChatMessageAttachment? + database.write { [weak self] session in guard let attachment = session.attachment(id: id) else { - completion(nil) - return + throw ClientError.AttachmentDoesNotExist(id: id) } if let temporaryURL = attachment.localURL { + guard self?.isAllowedToUpload(attachment.attachmentType, localURL: temporaryURL) == true else { + throw ClientError.AttachmentTypeDisallowed(id: id, attachmentType: attachment.attachmentType) + } do { let localURL = try attachmentStorage.storeAttachment(id: id, temporaryURL: temporaryURL) attachment.localURL = localURL @@ -122,14 +145,30 @@ class AttachmentQueueUploader: Worker { log.error("Could not copy attachment to local storage: \(error.localizedDescription)", subsystems: .offlineSupport) } } - - let model = attachment.asAnyModel() - + model = attachment.asAnyModel() + } completion: { error in DispatchQueue.main.async { - completion(model) + if let model { + completion(.success(model)) + } else if let error { + completion(.failure(error)) + } else { + completion(.failure(ClientError.Unknown("Incorrect completion handling in AttachmentQueueUploader"))) + } } } } + + private func isAllowedToUpload(_ attachmentType: AttachmentType, localURL: URL) -> Bool { + switch attachmentType { + case .image: + return queue.sync { imageUploadConfig?.isAllowed(localURL: localURL) ?? true } + case .audio, .file, .unknown, .video, .voiceRecording: + return queue.sync { fileUploadConfig?.isAllowed(localURL: localURL) ?? true } + default: + return true + } + } private func removePendingAttachment(with id: AttachmentId, result: Result) { _pendingAttachmentIDs.mutate { $0.remove(id) } @@ -311,7 +350,7 @@ extension AttachmentQueueUploader { for attachmentId: AttachmentId, continuation: CheckedContinuation ) { - continuationsQueue.async { + queue.async { self.continuations[attachmentId] = continuation } } @@ -320,7 +359,7 @@ extension AttachmentQueueUploader { for attachmentId: AttachmentId, result: Result ) { - continuationsQueue.async { + queue.async { guard let continuation = self.continuations.removeValue(forKey: attachmentId) else { return } continuation.resume(with: result) } diff --git a/Sources/StreamChatUI/Composer/ComposerVC.swift b/Sources/StreamChatUI/Composer/ComposerVC.swift index 0442019590..a9831f3bb0 100644 --- a/Sources/StreamChatUI/Composer/ComposerVC.swift +++ b/Sources/StreamChatUI/Composer/ComposerVC.swift @@ -405,8 +405,13 @@ open class ComposerVC: _ViewController, /// The view controller for selecting image attachments. open private(set) lazy var mediaPickerVC: UIViewController = { + let mediaTypes: [String] = { + let availableTypes = UIImagePickerController.availableMediaTypes(for: .savedPhotosAlbum) ?? ["public.image"] + let allowed = channelController?.client.appSettings?.imageUploadConfig.allowedUTITypes ?? [] + return allowed.isEmpty ? availableTypes : allowed + }() let picker = UIImagePickerController() - picker.mediaTypes = UIImagePickerController.availableMediaTypes(for: .savedPhotosAlbum) ?? ["public.image"] + picker.mediaTypes = mediaTypes picker.sourceType = .savedPhotosAlbum picker.delegate = self return picker @@ -414,17 +419,27 @@ open class ComposerVC: _ViewController, /// The View Controller for taking a picture. open private(set) lazy var cameraVC: UIViewController = { + let mediaTypes: [String] = { + let availableTypes = UIImagePickerController.availableMediaTypes(for: .camera) ?? ["public.image"] + let allowed = channelController?.client.appSettings?.imageUploadConfig.allowedUTITypes ?? [] + return allowed.isEmpty ? availableTypes : allowed + }() let camera = UIImagePickerController() camera.sourceType = .camera camera.modalPresentationStyle = .overFullScreen - camera.mediaTypes = UIImagePickerController.availableMediaTypes(for: .camera) ?? ["public.image"] + camera.mediaTypes = mediaTypes camera.delegate = self return camera }() /// The view controller for selecting file attachments. open private(set) lazy var filePickerVC: UIViewController = { - let picker = UIDocumentPickerViewController(documentTypes: ["public.item"], in: .import) + let documentTypes: [String] = { + let availableTypes = ["public.item"] + let allowed = channelController?.client.appSettings?.fileUploadConfig.allowedUTITypes ?? [] + return allowed.isEmpty ? availableTypes : allowed + }() + let picker = UIDocumentPickerViewController(documentTypes: documentTypes, in: .import) picker.delegate = self picker.allowsMultipleSelection = true return picker diff --git a/StreamChat.xcodeproj/project.pbxproj b/StreamChat.xcodeproj/project.pbxproj index 94b8a2ed72..7cd3404bdd 100644 --- a/StreamChat.xcodeproj/project.pbxproj +++ b/StreamChat.xcodeproj/project.pbxproj @@ -276,6 +276,7 @@ 4F5151982BC407ED001B7152 /* UserList_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F5151972BC407ED001B7152 /* UserList_Tests.swift */; }; 4F51519A2BC57C40001B7152 /* MessageState_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F5151992BC57C40001B7152 /* MessageState_Tests.swift */; }; 4F51519C2BC66FBE001B7152 /* Task+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F51519B2BC66FBE001B7152 /* Task+Extensions.swift */; }; + 4F6A77042D2FD0A00019CAF8 /* AppSettings_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F6A77032D2FD09A0019CAF8 /* AppSettings_Tests.swift */; }; 4F6AD5E42CABEAB6007E769C /* KeyPath+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F6AD5E32CABEAB0007E769C /* KeyPath+Extensions.swift */; }; 4F6AD5E52CABEAB6007E769C /* KeyPath+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F6AD5E32CABEAB0007E769C /* KeyPath+Extensions.swift */; }; 4F6B84102D008D6E005645B0 /* MemberUpdatePayload.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F6B840F2D008D5F005645B0 /* MemberUpdatePayload.swift */; }; @@ -3211,6 +3212,7 @@ 4F5151992BC57C40001B7152 /* MessageState_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageState_Tests.swift; sourceTree = ""; }; 4F51519B2BC66FBE001B7152 /* Task+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Task+Extensions.swift"; sourceTree = ""; }; 4F5758F02BB45B2F00D89A94 /* ChannelList_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ChannelList_Tests.swift; sourceTree = ""; }; + 4F6A77032D2FD09A0019CAF8 /* AppSettings_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppSettings_Tests.swift; sourceTree = ""; }; 4F6AD5E32CABEAB0007E769C /* KeyPath+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "KeyPath+Extensions.swift"; sourceTree = ""; }; 4F6B840F2D008D5F005645B0 /* MemberUpdatePayload.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MemberUpdatePayload.swift; sourceTree = ""; }; 4F73F3972B91BD3000563CD9 /* MessageState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageState.swift; sourceTree = ""; }; @@ -7249,6 +7251,7 @@ A364D0A027D0C8690029857A /* Models */ = { isa = PBXGroup; children = ( + 4F6A77032D2FD09A0019CAF8 /* AppSettings_Tests.swift */, 8A5D3EF824AF749200E2FE35 /* ChannelId_Tests.swift */, 84FD350727FD8BE300D68D85 /* ChatChannel_Tests.swift */, 64C80614262EDA9600B1F7AD /* ChatMessage_Tests.swift */, @@ -12012,6 +12015,7 @@ 8A0D649D24E579F70017A3C0 /* GuestUserTokenPayload_Tests.swift in Sources */, AD9490622BF66D1E00E69224 /* ThreadsRepository_Mock.swift in Sources */, 8A0C3BE224C1F74200CAFD19 /* MessageEvents_Tests.swift in Sources */, + 4F6A77042D2FD0A00019CAF8 /* AppSettings_Tests.swift in Sources */, 7990503224CEEAA600689CDC /* MessageDTO_Tests.swift in Sources */, 88D85DAB252F3C2A00AE1030 /* MemberListController_Tests.swift in Sources */, 88DA577E2631D73800FA8C53 /* ChannelMuteDTO_Tests.swift in Sources */, diff --git a/Tests/StreamChatTests/Models/AppSettings_Tests.swift b/Tests/StreamChatTests/Models/AppSettings_Tests.swift new file mode 100644 index 0000000000..a0c5a4ad38 --- /dev/null +++ b/Tests/StreamChatTests/Models/AppSettings_Tests.swift @@ -0,0 +1,139 @@ +// +// Copyright © 2025 Stream.io Inc. All rights reserved. +// + +@testable import StreamChat +@testable import StreamChatTestTools +import XCTest + +final class AppSettings_Tests: XCTestCase { + // MARK: - Upload Config Path Extensions + + func test_uploadConfig_empty_pathExtension() { + let config = AppSettings.UploadConfig.mock() + let allowedURLs = Self.testFileURLs.filter { config.isAllowed(localURL: $0) } + XCTAssertEqual(allowedURLs.count, Self.testFileURLs.count) + } + + func test_uploadConfig_noPathExtension() { + let config = AppSettings.UploadConfig.mock() + let url = URL(fileURLWithPath: "/A/file") + XCTAssertTrue(config.isAllowed(localURL: url)) + } + + func test_uploadConfig_noPathExtension_allowSet() { + let config = AppSettings.UploadConfig.mock( + allowedFileExtensions: [".txt"], + allowedMimeTypes: ["text/plain"] + ) + let url = URL(fileURLWithPath: "/A/file") + XCTAssertTrue(config.isAllowed(localURL: url)) + } + + func test_uploadConfig_allowed_pathExtension() { + let config = AppSettings.UploadConfig.mock( + allowedFileExtensions: [".M4A", ".txt", "zip"] + ) + let allowedURLs = Self.testFileURLs.filter { config.isAllowed(localURL: $0) } + XCTAssertEqual(["m4a", "txt", "zip"], allowedURLs.map(\.pathExtension).sorted()) + } + + func test_uploadConfig_blocked_pathExtension() { + let config = AppSettings.UploadConfig.mock( + blockedFileExtensions: [".M4A", ".txt", "zip"] + ) + let blockedURLs = Self.testFileURLs.filter { !config.isAllowed(localURL: $0) } + XCTAssertEqual(["m4a", "txt", "zip"], blockedURLs.map(\.pathExtension).sorted()) + } + + func test_uploadConfig_invalid_allowedAndBlockedEqual_pathExtension() { + // Should not happen, but when it does, nothing goes through + let config = AppSettings.UploadConfig.mock( + allowedFileExtensions: [".txt"], + blockedFileExtensions: [".txt"] + ) + let allowedURLs = Self.testFileURLs.filter { config.isAllowed(localURL: $0) } + XCTAssertEqual([], allowedURLs.map(\.pathExtension).sorted()) + } + + func test_uploadConfig_invalid_allowedAndBlockedMixed_pathExtension() { + // Should not happen, only either of these should be defined, but when both are, blocked overrides allowed + let config = AppSettings.UploadConfig.mock( + allowedFileExtensions: [".m4a", ".txt"], + blockedFileExtensions: [".m4a"] + ) + let allowedURLs = Self.testFileURLs.filter { config.isAllowed(localURL: $0) } + XCTAssertEqual(["txt"], allowedURLs.map(\.pathExtension).sorted()) + } + + // MARK: - Upload Config MIME Types + + func test_uploadConfig_allowed_mimeType() { + let config = AppSettings.UploadConfig.mock( + allowedMimeTypes: ["application/ZIP", "text/plain"] + ) + let allowedURLs = Self.testFileURLs.filter { config.isAllowed(localURL: $0) } + XCTAssertEqual(["txt", "zip"], allowedURLs.map(\.pathExtension).sorted()) + } + + func test_uploadConfig_blocked_mimeType() { + let config = AppSettings.UploadConfig.mock( + blockedMimeTypes: ["application/zip", "texT/plain"] + ) + let blockedURLs = Self.testFileURLs.filter { !config.isAllowed(localURL: $0) } + XCTAssertEqual(["txt", "zip"], blockedURLs.map(\.pathExtension).sorted()) + } + + func test_uploadConfig_invalid_allowedAndBlockedEqual_mimeTypes() { + // Should not happen, but when it does, nothing goes through + let config = AppSettings.UploadConfig.mock( + allowedMimeTypes: ["text/plain"], + blockedMimeTypes: ["text/plain"] + ) + let allowedURLs = Self.testFileURLs.filter { config.isAllowed(localURL: $0) } + XCTAssertEqual([], allowedURLs.map(\.pathExtension).sorted()) + } + + func test_uploadConfig_invalid_allowedAndBlockedMixed_mimeTypes() { + // Should not happen, only either of these should be defined, but when both are, blocked overrides allowed + let config = AppSettings.UploadConfig.mock( + allowedMimeTypes: ["audio/mp4", "text/plain"], + blockedMimeTypes: ["audio/mp4"] + ) + let allowedURLs = Self.testFileURLs.filter { config.isAllowed(localURL: $0) } + XCTAssertEqual(["txt"], allowedURLs.map(\.pathExtension).sorted()) + } + + // MARK: - Upload Config UTI Type Conversion + + func test_uploadConfig_allowed_utiType() { + let config = AppSettings.UploadConfig.mock( + allowedFileExtensions: [".m4a"], + allowedMimeTypes: ["application/ZIP", "text/plain"] + ) + XCTAssertEqual(["com.apple.m4a-audio", "public.plain-text", "public.zip-archive"], config.allowedUTITypes.sorted()) + } + + func test_uploadConfig_blocked_utiType() { + let config = AppSettings.UploadConfig.mock( + allowedFileExtensions: [".7z"], + allowedMimeTypes: ["video/mp4"] + ) + XCTAssertEqual(["org.7-zip.7-zip-archive", "public.mpeg-4"], config.allowedUTITypes.sorted()) + } + + // MARK: - Test Data + + static var testFileURLs: [URL] = { + let pathExtensions = AttachmentFileType.allCases.map(\.rawValue) + return pathExtensions.map { pathExtension in + URL(fileURLWithPath: "/A/file.\(pathExtension)") + } + }() +} + +private extension URL { + static func fileName(_ fileName: String) -> URL { + URL(fileURLWithPath: "/A/\(fileName)") + } +} diff --git a/Tests/StreamChatTests/Workers/Background/AttachmentQueueUploader_Tests.swift b/Tests/StreamChatTests/Workers/Background/AttachmentQueueUploader_Tests.swift index 8ce1124f7a..510a5f386f 100644 --- a/Tests/StreamChatTests/Workers/Background/AttachmentQueueUploader_Tests.swift +++ b/Tests/StreamChatTests/Workers/Background/AttachmentQueueUploader_Tests.swift @@ -643,6 +643,123 @@ final class AttachmentQueueUploader_Tests: XCTestCase { attachmentPayloads.count ) } + + // MARK: - App Settings + + func test_uploadedAttachmentWithAllowedPathExtension() throws { + try uploadAttachmentsWithAppSettings( + AppSettings.mock( + fileUploadConfig: .mock(allowedFileExtensions: [".txt"]), + imageUploadConfig: .mock(allowedFileExtensions: [".jpg"]) + ), + expectedState: [ + "txt": .pendingUpload, + "jpg": .pendingUpload, + "mov": .uploadingFailed, + "m4a": .uploadingFailed + ] + ) + } + + func test_uploadedAttachmentWithAllowedMimeTypes() throws { + try uploadAttachmentsWithAppSettings( + AppSettings.mock( + fileUploadConfig: .mock(allowedMimeTypes: ["video/quicktime"]), + imageUploadConfig: .mock(allowedMimeTypes: ["image/jpeg"]) + ), + expectedState: [ + "txt": .uploadingFailed, + "jpg": .pendingUpload, + "mov": .pendingUpload, + "m4a": .uploadingFailed + ] + ) + } + + func test_uploadedAttachmentWithBlockedPathExtension() throws { + try uploadAttachmentsWithAppSettings( + AppSettings.mock( + fileUploadConfig: .mock(blockedFileExtensions: [".txt"]), + imageUploadConfig: .mock(blockedFileExtensions: [".jpg"]) + ), + expectedState: [ + "txt": .uploadingFailed, + "jpg": .uploadingFailed, + "mov": .pendingUpload, + "m4a": .pendingUpload + ] + ) + } + + func test_uploadedAttachmentWithBlockedMimeTypes() throws { + try uploadAttachmentsWithAppSettings( + AppSettings.mock( + fileUploadConfig: .mock(blockedMimeTypes: ["video/quicktime"]), + imageUploadConfig: .mock(blockedMimeTypes: ["image/jpeg"]) + ), + expectedState: [ + "txt": .pendingUpload, + "jpg": .uploadingFailed, + "mov": .uploadingFailed, + "m4a": .pendingUpload + ] + ) + } + + func uploadAttachmentsWithAppSettings(_ appSettings: AppSettings, expectedState: Dictionary) throws { + queueUploader.setAppSettings(appSettings) + + let cid: ChannelId = .unique + let messageId: MessageId = .unique + + // Create channel in the database. + try database.createChannel(cid: cid, withMessages: false) + // Create message in the database. + try database.createMessage(id: messageId, cid: cid, localState: .pendingSend) + + let attachmentPayloads: [AnyAttachmentPayload] = [ + .mockFile, // txt + .mockImage, // jpg + try AnyAttachmentPayload(localFileURL: try createLocalFile(fileName: "myvideo.mov"), attachmentType: .video), + try AnyAttachmentPayload(localFileURL: try createLocalFile(fileName: "myaudio.m4a"), attachmentType: .audio) + ] + + for (index, envelope) in attachmentPayloads.enumerated() { + let attachmentId = AttachmentId(cid: cid, messageId: messageId, index: index) + // Seed attachment in `.pendingUpload` state to the database. + try database.writeSynchronously { session in + try session.createNewAttachment(attachment: envelope, id: attachmentId) + } + } + + let expectation = XCTestExpectation() + let observer = StateLayerDatabaseObserver( + database: database, + fetchRequest: MessageDTO.message(withID: messageId), + itemCreator: { try $0.asModel() } + ) + try observer.startObserving { message, _ in + guard let message else { return } + let states = Dictionary(uniqueKeysWithValues: message.allAttachments.map { ($0.uploadingState?.localFileURL.pathExtension, $0.uploadingState?.state) }) + guard states == expectedState else { return } + expectation.fulfill() + } + wait(for: [expectation], timeout: defaultTimeout) + XCTAssertEqual(messageId, observer.item?.id) + } + + // MARK: - Test Data + + func createLocalFile(fileName: String) throws -> URL { + let fileManager = FileManager.default + let fileContent = "This is the file content" + let fileData = try XCTUnwrap(fileContent.data(using: .utf8)) + let folderURL = fileManager.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try fileManager.createDirectory(at: folderURL, withIntermediateDirectories: true) + let temporaryFileURL = folderURL.appendingPathComponent(fileName) + try fileData.write(to: temporaryFileURL) + return temporaryFileURL + } } private extension URL { From c585bceb6cdabf2292831612be9a5f2620577d87 Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Mon, 13 Jan 2025 10:03:09 +0200 Subject: [PATCH 2/7] Remove unused extension --- Tests/StreamChatTests/Models/AppSettings_Tests.swift | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Tests/StreamChatTests/Models/AppSettings_Tests.swift b/Tests/StreamChatTests/Models/AppSettings_Tests.swift index a0c5a4ad38..ad13d1bee7 100644 --- a/Tests/StreamChatTests/Models/AppSettings_Tests.swift +++ b/Tests/StreamChatTests/Models/AppSettings_Tests.swift @@ -131,9 +131,3 @@ final class AppSettings_Tests: XCTestCase { } }() } - -private extension URL { - static func fileName(_ fileName: String) -> URL { - URL(fileURLWithPath: "/A/\(fileName)") - } -} From 75d700cd54408e37fe152599282d61b3f47e7098 Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Mon, 13 Jan 2025 10:44:44 +0200 Subject: [PATCH 3/7] Add CHANGELOG entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cbf1c9110..be73448ccb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## StreamChat ### ✅ Added +- Use `AppSettings.fileUploadConfig` and `AppSettings.imageUploadConfig` for blocking attachment uploads [#3556](https://github.com/GetStream/stream-chat-swift/pull/3556/) - Add `FilterKey.disabled` and `ChatChannel.isDisabled` [#3546](https://github.com/GetStream/stream-chat-swift/pull/3546) - Add `ImageAttachmentPayload.file` for setting `file_size` and `mime_type` for image attachments [#3548](https://github.com/GetStream/stream-chat-swift/pull/3548) ### 🐞 Fixed From a22a247e4eab311e8c99911f15e43634cfb877fa Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Mon, 13 Jan 2025 16:07:55 +0200 Subject: [PATCH 4/7] Changelog entry for StreamChatUI --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be73448ccb..6680406911 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## StreamChat ### ✅ Added -- Use `AppSettings.fileUploadConfig` and `AppSettings.imageUploadConfig` for blocking attachment uploads [#3556](https://github.com/GetStream/stream-chat-swift/pull/3556/) +- Use `AppSettings.fileUploadConfig` and `AppSettings.imageUploadConfig` for blocking attachment uploads [#3556](https://github.com/GetStream/stream-chat-swift/pull/3556) - Add `FilterKey.disabled` and `ChatChannel.isDisabled` [#3546](https://github.com/GetStream/stream-chat-swift/pull/3546) - Add `ImageAttachmentPayload.file` for setting `file_size` and `mime_type` for image attachments [#3548](https://github.com/GetStream/stream-chat-swift/pull/3548) ### 🐞 Fixed @@ -17,6 +17,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### 🔄 Changed - Deprecate `ImageAttachmentPayload.init(title:imageRemoteURL:originalWidth:originalHeight:extraData:)` in favor of `ImageAttachmentPayload.init(title:imageRemoteURL:file:originalWidth:originalHeight:extraData:)` [#3548](https://github.com/GetStream/stream-chat-swift/pull/3548) +## StreamChatUI +### 🔄 Changed +- Set supported media types based on `AppSettings` in `ComposerVC.mediaPickerVC`, `ComposerVC.filePickerVC`, and `ComposerVC.mediaPickerVC` [#3556](https://github.com/GetStream/stream-chat-swift/pull/3556) + # [4.69.0](https://github.com/GetStream/stream-chat-swift/releases/tag/4.69.0) _December 18, 2024_ From 71011676ec225406a766b34a38eba008c1128b27 Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Tue, 14 Jan 2025 11:35:31 +0200 Subject: [PATCH 5/7] Do not set media types for photo picker because it can easily lead to an exception --- CHANGELOG.md | 2 +- Sources/StreamChatUI/Composer/ComposerVC.swift | 14 ++------------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6680406911..3d895fcdaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## StreamChatUI ### 🔄 Changed -- Set supported media types based on `AppSettings` in `ComposerVC.mediaPickerVC`, `ComposerVC.filePickerVC`, and `ComposerVC.mediaPickerVC` [#3556](https://github.com/GetStream/stream-chat-swift/pull/3556) +- Set supported media types based on `AppSettings` in `ComposerVC.filePickerVC` [#3556](https://github.com/GetStream/stream-chat-swift/pull/3556) # [4.69.0](https://github.com/GetStream/stream-chat-swift/releases/tag/4.69.0) _December 18, 2024_ diff --git a/Sources/StreamChatUI/Composer/ComposerVC.swift b/Sources/StreamChatUI/Composer/ComposerVC.swift index a9831f3bb0..af61f979eb 100644 --- a/Sources/StreamChatUI/Composer/ComposerVC.swift +++ b/Sources/StreamChatUI/Composer/ComposerVC.swift @@ -405,13 +405,8 @@ open class ComposerVC: _ViewController, /// The view controller for selecting image attachments. open private(set) lazy var mediaPickerVC: UIViewController = { - let mediaTypes: [String] = { - let availableTypes = UIImagePickerController.availableMediaTypes(for: .savedPhotosAlbum) ?? ["public.image"] - let allowed = channelController?.client.appSettings?.imageUploadConfig.allowedUTITypes ?? [] - return allowed.isEmpty ? availableTypes : allowed - }() let picker = UIImagePickerController() - picker.mediaTypes = mediaTypes + picker.mediaTypes = UIImagePickerController.availableMediaTypes(for: .savedPhotosAlbum) ?? ["public.image"] picker.sourceType = .savedPhotosAlbum picker.delegate = self return picker @@ -419,15 +414,10 @@ open class ComposerVC: _ViewController, /// The View Controller for taking a picture. open private(set) lazy var cameraVC: UIViewController = { - let mediaTypes: [String] = { - let availableTypes = UIImagePickerController.availableMediaTypes(for: .camera) ?? ["public.image"] - let allowed = channelController?.client.appSettings?.imageUploadConfig.allowedUTITypes ?? [] - return allowed.isEmpty ? availableTypes : allowed - }() let camera = UIImagePickerController() camera.sourceType = .camera camera.modalPresentationStyle = .overFullScreen - camera.mediaTypes = mediaTypes + camera.mediaTypes = UIImagePickerController.availableMediaTypes(for: .camera) ?? ["public.image"] camera.delegate = self return camera }() From 783cf05f0e09c99415fa491c1015184a32c08e7d Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Tue, 14 Jan 2025 11:39:04 +0200 Subject: [PATCH 6/7] Documentation for new public functions --- Sources/StreamChat/Models/AppSettings.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Sources/StreamChat/Models/AppSettings.swift b/Sources/StreamChat/Models/AppSettings.swift index a997c465ab..d43c4373ae 100644 --- a/Sources/StreamChat/Models/AppSettings.swift +++ b/Sources/StreamChat/Models/AppSettings.swift @@ -63,12 +63,14 @@ extension AppSettingsPayload.UploadConfigPayload { extension AppSettings.UploadConfig { // MARK: - UTI Validation - + + /// Returns an array of allowed UTI identifiers based on allowed mime types and file extensions. public var allowedUTITypes: [String] { allowedMimeTypes.compactMap { $0.utiType(mime: true) } + allowedFileExtensions.compactMap { $0.utiType(mime: false) } } + /// Returns an array of blocked UTI identifiers based on allowed mime types and file extensions. public var blockedUTITypes: [String] { blockedMimeTypes.compactMap { $0.utiType(mime: true) } + blockedFileExtensions.compactMap { $0.utiType(mime: false) } From 5df9fb9b9ab0e7885d7ad05dc54808d9853f5a6f Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Tue, 14 Jan 2025 12:39:33 +0200 Subject: [PATCH 7/7] Validate attachment after preparing if for making sure the local file was copied to the attachment storage --- .../Database/DTOs/AttachmentDTO.swift | 6 ++--- .../Background/AttachmentQueueUploader.swift | 24 ++++++++++++++----- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift b/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift index 52dcfbb679..2e8b7aa99e 100644 --- a/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift +++ b/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift @@ -376,9 +376,9 @@ extension ClientError { } } - final class AttachmentTypeDisallowed: ClientError { - init(id: AttachmentId, attachmentType: AttachmentType) { - super.init("`AttachmentDTO` with \(id) has type \(attachmentType) which is marked as disallowed on the Stream dashboard.") + final class AttachmentUploadBlocked: ClientError { + init(id: AttachmentId, attachmentType: AttachmentType, pathExtension: String) { + super.init("`AttachmentDTO` with \(id) and type \(attachmentType) and path extension \(pathExtension) is blocked on the Stream dashboard.") } } diff --git a/Sources/StreamChat/Workers/Background/AttachmentQueueUploader.swift b/Sources/StreamChat/Workers/Background/AttachmentQueueUploader.swift index 7b1b7ab898..c05791ec9d 100644 --- a/Sources/StreamChat/Workers/Background/AttachmentQueueUploader.swift +++ b/Sources/StreamChat/Workers/Background/AttachmentQueueUploader.swift @@ -129,15 +129,13 @@ class AttachmentQueueUploader: Worker { private func prepareAttachmentForUpload(with id: AttachmentId, completion: @escaping (Result) -> Void) { let attachmentStorage = self.attachmentStorage var model: AnyChatMessageAttachment? - database.write { [weak self] session in + var attachmentLocalURL: URL? + database.write { session in guard let attachment = session.attachment(id: id) else { throw ClientError.AttachmentDoesNotExist(id: id) } if let temporaryURL = attachment.localURL { - guard self?.isAllowedToUpload(attachment.attachmentType, localURL: temporaryURL) == true else { - throw ClientError.AttachmentTypeDisallowed(id: id, attachmentType: attachment.attachmentType) - } do { let localURL = try attachmentStorage.storeAttachment(id: id, temporaryURL: temporaryURL) attachment.localURL = localURL @@ -145,11 +143,25 @@ class AttachmentQueueUploader: Worker { log.error("Could not copy attachment to local storage: \(error.localizedDescription)", subsystems: .offlineSupport) } } + attachmentLocalURL = attachment.localURL model = attachment.asAnyModel() - } completion: { error in + } completion: { [weak self] error in DispatchQueue.main.async { if let model { - completion(.success(model)) + // Attachment uploading state should be validated after preparing the local file (for ensuring the local file persists for retry) + if let attachmentLocalURL, self?.isAllowedToUpload(model.type, localURL: attachmentLocalURL) == false { + completion( + .failure( + ClientError.AttachmentUploadBlocked( + id: id, + attachmentType: model.type, + pathExtension: attachmentLocalURL.pathExtension + ) + ) + ) + } else { + completion(.success(model)) + } } else if let error { completion(.failure(error)) } else {