Skip to content

Commit

Permalink
Warn and block sending on verification violation (#3679)
Browse files Browse the repository at this point in the history 
* feat(crypto): Warn and block sending on verification violation

* fixup: Fix ComposerToolbar previews

* fixup! add ComposerToolBarViewModelTests for canSend

* add new preview tests for verification violations

* Use `deferFulfillment`s in the unit tests.

---------

Co-authored-by: Stefan Ceriu <stefanc@matrix.org>
  • Loading branch information
@BillCarsonFr @stefanceriu
BillCarsonFr and stefanceriu authored Jan 20, 2025
1 parent 0fd8df5 commit f20847f
Show file tree
Hide file tree
Showing 21 changed files with 419 additions and 22 deletions.
70 changes: 70 additions & 0 deletions ElementX/Sources/Mocks/Generated/GeneratedMocks.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4682,6 +4682,76 @@ class ClientProxyMock: ClientProxyProtocol, @unchecked Sendable {
return pinUserIdentityReturnValue
}
}
//MARK: - withdrawUserIdentityVerification

var withdrawUserIdentityVerificationUnderlyingCallsCount = 0
var withdrawUserIdentityVerificationCallsCount: Int {
get {
if Thread.isMainThread {
return withdrawUserIdentityVerificationUnderlyingCallsCount
} else {
var returnValue: Int? = nil
DispatchQueue.main.sync {
returnValue = withdrawUserIdentityVerificationUnderlyingCallsCount
}

return returnValue!
}
}
set {
if Thread.isMainThread {
withdrawUserIdentityVerificationUnderlyingCallsCount = newValue
} else {
DispatchQueue.main.sync {
withdrawUserIdentityVerificationUnderlyingCallsCount = newValue
}
}
}
}
var withdrawUserIdentityVerificationCalled: Bool {
return withdrawUserIdentityVerificationCallsCount > 0
}
var withdrawUserIdentityVerificationReceivedUserID: String?
var withdrawUserIdentityVerificationReceivedInvocations: [String] = []

var withdrawUserIdentityVerificationUnderlyingReturnValue: Result<Void, ClientProxyError>!
var withdrawUserIdentityVerificationReturnValue: Result<Void, ClientProxyError>! {
get {
if Thread.isMainThread {
return withdrawUserIdentityVerificationUnderlyingReturnValue
} else {
var returnValue: Result<Void, ClientProxyError>? = nil
DispatchQueue.main.sync {
returnValue = withdrawUserIdentityVerificationUnderlyingReturnValue
}

return returnValue!
}
}
set {
if Thread.isMainThread {
withdrawUserIdentityVerificationUnderlyingReturnValue = newValue
} else {
DispatchQueue.main.sync {
withdrawUserIdentityVerificationUnderlyingReturnValue = newValue
}
}
}
}
var withdrawUserIdentityVerificationClosure: ((String) async -> Result<Void, ClientProxyError>)?

func withdrawUserIdentityVerification(_ userID: String) async -> Result<Void, ClientProxyError> {
withdrawUserIdentityVerificationCallsCount += 1
withdrawUserIdentityVerificationReceivedUserID = userID
DispatchQueue.main.async {
self.withdrawUserIdentityVerificationReceivedInvocations.append(userID)
}
if let withdrawUserIdentityVerificationClosure = withdrawUserIdentityVerificationClosure {
return await withdrawUserIdentityVerificationClosure(userID)
} else {
return withdrawUserIdentityVerificationReturnValue
}
}
//MARK: - resetIdentity

var resetIdentityUnderlyingCallsCount = 0
Expand Down
6 changes: 6 additions & 0 deletions ElementX/Sources/Screens/RoomScreen/ComposerToolbar/ComposerToolbarModels.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ enum ComposerAttachmentType {
struct ComposerToolbarViewState: BindableState {
var composerMode: ComposerMode = .default
var composerEmpty = true
/// Could be false if sending is disabled in the room
var canSend = true
var suggestions: [SuggestionItem] = []
var audioPlayerState: AudioPlayerState
var audioRecorderState: AudioRecorderState
Expand Down Expand Up @@ -97,6 +99,10 @@ struct ComposerToolbarViewState: BindableState {
}

var sendButtonDisabled: Bool {
if !canSend {
return true
}

if case .previewVoiceMessage = composerMode {
return false
}
Expand Down
36 changes: 36 additions & 0 deletions ElementX/Sources/Screens/RoomScreen/ComposerToolbar/ComposerToolbarViewModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
private var initialText: String?
private let wysiwygViewModel: WysiwygComposerViewModel
private let completionSuggestionService: CompletionSuggestionServiceProtocol
private let roomProxy: JoinedRoomProxyProtocol
private let analyticsService: AnalyticsService
private let draftService: ComposerDraftServiceProtocol
private var identityPinningViolations = [String: RoomMemberProxyProtocol]()

private let mentionBuilder: MentionBuilderProtocol
private let attributedStringBuilder: AttributedStringBuilderProtocol
Expand All @@ -43,6 +45,7 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
private var replyLoadingTask: Task<Void, Never>?

init(initialText: String? = nil,
roomProxy: JoinedRoomProxyProtocol,
wysiwygViewModel: WysiwygComposerViewModel,
completionSuggestionService: CompletionSuggestionServiceProtocol,
mediaProvider: MediaProviderProtocol,
Expand All @@ -53,6 +56,7 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
self.wysiwygViewModel = wysiwygViewModel
self.completionSuggestionService = completionSuggestionService
self.analyticsService = analyticsService
self.roomProxy = roomProxy
draftService = composerDraftService

mentionBuilder = MentionBuilder()
Expand Down Expand Up @@ -120,6 +124,19 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool

setupMentionsHandling(mentionDisplayHelper: mentionDisplayHelper)
focusComposerIfHardwareKeyboardConnected()

let identityStatusChangesPublisher = roomProxy.identityStatusChangesPublisher.receive(on: DispatchQueue.main)

Task { [weak self] in
for await changes in identityStatusChangesPublisher.values {
guard !Task.isCancelled else {
return
}

await self?.processIdentityStatusChanges(changes)
}
}
.store(in: &cancellables)
}

// MARK: - Public
Expand Down Expand Up @@ -477,6 +494,25 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
}
}
}

private func processIdentityStatusChanges(_ changes: [IdentityStatusChange]) async {
for change in changes {
switch change.changedTo {
case .verificationViolation:
guard case let .success(member) = await roomProxy.getMember(userID: change.userId) else {
MXLog.error("Failed retrieving room member for identity status change: \(change)")
continue
}

identityPinningViolations[change.userId] = member
default:
// clear
identityPinningViolations[change.userId] = nil
}
}

state.canSend = identityPinningViolations.isEmpty
}

private func set(mode: ComposerMode) {
if state.composerMode.isLoadingReply, state.composerMode.replyEventID != mode.replyEventID {
Expand Down
44 changes: 38 additions & 6 deletions ElementX/Sources/Screens/RoomScreen/ComposerToolbar/View/ComposerToolbar.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
// Please see LICENSE files in the repository root for full details.
//

import Combine
import Compound
import MatrixRustSDK
import SwiftUI
import WysiwygComposer

Expand Down Expand Up @@ -44,6 +46,7 @@ struct ComposerToolbar: View {
.offset(y: -frame.height)
}
}
.disabled(!context.viewState.canSend)
.alert(item: $context.alertInfo)
}

Expand Down Expand Up @@ -297,7 +300,8 @@ struct ComposerToolbar: View {

struct ComposerToolbar_Previews: PreviewProvider, TestablePreview {
static let wysiwygViewModel = WysiwygComposerViewModel()
static let composerViewModel = ComposerToolbarViewModel(wysiwygViewModel: wysiwygViewModel,
static let composerViewModel = ComposerToolbarViewModel(roomProxy: JoinedRoomProxyMock(.init()),
wysiwygViewModel: wysiwygViewModel,
completionSuggestionService: CompletionSuggestionServiceMock(configuration: .init(suggestions: suggestions)),
mediaProvider: MediaProviderMock(configuration: .init()),
mentionDisplayHelper: ComposerMentionDisplayHelper.mock,
Expand Down Expand Up @@ -331,14 +335,20 @@ struct ComposerToolbar_Previews: PreviewProvider, TestablePreview {
ComposerToolbar.replyLoadingPreviewMock(isLoading: false)
}
.previewDisplayName("Reply")

VStack(spacing: 8) {
ComposerToolbar.disabledPreviewMock()
}
.previewDisplayName("Disabled")
}
}

extension ComposerToolbar {
static func mock(focused: Bool = true) -> ComposerToolbar {
let wysiwygViewModel = WysiwygComposerViewModel()
var composerViewModel: ComposerToolbarViewModel {
let model = ComposerToolbarViewModel(wysiwygViewModel: wysiwygViewModel,
let model = ComposerToolbarViewModel(roomProxy: JoinedRoomProxyMock(.init()),
wysiwygViewModel: wysiwygViewModel,
completionSuggestionService: CompletionSuggestionServiceMock(configuration: .init()),
mediaProvider: MediaProviderMock(configuration: .init()),
mentionDisplayHelper: ComposerMentionDisplayHelper.mock,
Expand All @@ -355,7 +365,8 @@ extension ComposerToolbar {
static func textWithVoiceMessage(focused: Bool = true) -> ComposerToolbar {
let wysiwygViewModel = WysiwygComposerViewModel()
var composerViewModel: ComposerToolbarViewModel {
let model = ComposerToolbarViewModel(wysiwygViewModel: wysiwygViewModel,
let model = ComposerToolbarViewModel(roomProxy: JoinedRoomProxyMock(.init()),
wysiwygViewModel: wysiwygViewModel,
completionSuggestionService: CompletionSuggestionServiceMock(configuration: .init()),
mediaProvider: MediaProviderMock(configuration: .init()),
mentionDisplayHelper: ComposerMentionDisplayHelper.mock,
Expand All @@ -372,7 +383,8 @@ extension ComposerToolbar {
static func voiceMessageRecordingMock() -> ComposerToolbar {
let wysiwygViewModel = WysiwygComposerViewModel()
var composerViewModel: ComposerToolbarViewModel {
let model = ComposerToolbarViewModel(wysiwygViewModel: wysiwygViewModel,
let model = ComposerToolbarViewModel(roomProxy: JoinedRoomProxyMock(.init()),
wysiwygViewModel: wysiwygViewModel,
completionSuggestionService: CompletionSuggestionServiceMock(configuration: .init()),
mediaProvider: MediaProviderMock(configuration: .init()),
mentionDisplayHelper: ComposerMentionDisplayHelper.mock,
Expand All @@ -390,7 +402,8 @@ extension ComposerToolbar {
let wysiwygViewModel = WysiwygComposerViewModel()
let waveformData: [Float] = Array(repeating: 1.0, count: 1000)
var composerViewModel: ComposerToolbarViewModel {
let model = ComposerToolbarViewModel(wysiwygViewModel: wysiwygViewModel,
let model = ComposerToolbarViewModel(roomProxy: JoinedRoomProxyMock(.init()),
wysiwygViewModel: wysiwygViewModel,
completionSuggestionService: CompletionSuggestionServiceMock(configuration: .init()),
mediaProvider: MediaProviderMock(configuration: .init()),
mentionDisplayHelper: ComposerMentionDisplayHelper.mock,
Expand All @@ -411,7 +424,8 @@ extension ComposerToolbar {
static func replyLoadingPreviewMock(isLoading: Bool) -> ComposerToolbar {
let wysiwygViewModel = WysiwygComposerViewModel()
var composerViewModel: ComposerToolbarViewModel {
let model = ComposerToolbarViewModel(wysiwygViewModel: wysiwygViewModel,
let model = ComposerToolbarViewModel(roomProxy: JoinedRoomProxyMock(.init()),
wysiwygViewModel: wysiwygViewModel,
completionSuggestionService: CompletionSuggestionServiceMock(configuration: .init()),
mediaProvider: MediaProviderMock(configuration: .init()),
mentionDisplayHelper: ComposerMentionDisplayHelper.mock,
Expand All @@ -430,4 +444,22 @@ extension ComposerToolbar {
wysiwygViewModel: wysiwygViewModel,
keyCommands: [])
}

static func disabledPreviewMock() -> ComposerToolbar {
let wysiwygViewModel = WysiwygComposerViewModel()
var composerViewModel: ComposerToolbarViewModel {
let model = ComposerToolbarViewModel(roomProxy: JoinedRoomProxyMock(.init()),
wysiwygViewModel: wysiwygViewModel,
completionSuggestionService: CompletionSuggestionServiceMock(configuration: .init()),
mediaProvider: MediaProviderMock(configuration: .init()),
mentionDisplayHelper: ComposerMentionDisplayHelper.mock,
analyticsService: ServiceLocator.shared.analytics,
composerDraftService: ComposerDraftServiceMock())
model.state.canSend = false
return model
}
return ComposerToolbar(context: composerViewModel.context,
wysiwygViewModel: wysiwygViewModel,
keyCommands: [])
}
}
8 changes: 7 additions & 1 deletion ElementX/Sources/Screens/RoomScreen/ComposerToolbar/View/RoomAttachmentPicker.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import WysiwygComposer
struct RoomAttachmentPicker: View {
@ObservedObject var context: ComposerToolbarViewModel.Context

@Environment(\.isEnabled) private var isEnabled

var body: some View {
// Use a menu instead of the popover/sheet shown in Figma because overriding the colour scheme
// results in a rendering bug on 17.1: https://github.com/element-hq/element-x-ios/issues/2157
Expand All @@ -20,6 +22,9 @@ struct RoomAttachmentPicker: View {
} label: {
CompoundIcon(asset: Asset.Images.composerAttachment, size: .custom(30), relativeTo: .compound.headingLG)
.scaledPadding(7, relativeTo: .compound.headingLG)
.foregroundColor(
isEnabled ? .compound.iconPrimary : .compound.iconDisabled
)
}
.buttonStyle(RoomAttachmentPickerButtonStyle())
.accessibilityLabel(L10n.actionAddToTimeline)
Expand Down Expand Up @@ -81,7 +86,8 @@ private struct RoomAttachmentPickerButtonStyle: ButtonStyle {
}

struct RoomAttachmentPicker_Previews: PreviewProvider, TestablePreview {
static let viewModel = ComposerToolbarViewModel(wysiwygViewModel: WysiwygComposerViewModel(),
static let viewModel = ComposerToolbarViewModel(roomProxy: JoinedRoomProxyMock(.init()),
wysiwygViewModel: WysiwygComposerViewModel(),
completionSuggestionService: CompletionSuggestionServiceMock(configuration: .init()),
mediaProvider: MediaProviderMock(configuration: .init()),
mentionDisplayHelper: ComposerMentionDisplayHelper.mock,
Expand Down
6 changes: 5 additions & 1 deletion ElementX/Sources/Screens/RoomScreen/ComposerToolbar/View/VoiceMessageRecordingButton.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ enum VoiceMessageRecordingButtonMode {
}

struct VoiceMessageRecordingButton: View {
@Environment(\.isEnabled) private var isEnabled

let mode: VoiceMessageRecordingButtonMode
var startRecording: (() -> Void)?
var stopRecording: (() -> Void)?
Expand All @@ -33,7 +35,9 @@ struct VoiceMessageRecordingButton: View {
switch mode {
case .idle:
CompoundIcon(\.micOn, size: .medium, relativeTo: .compound.headingLG)
.foregroundColor(.compound.iconSecondary)
.foregroundColor(
isEnabled ? .compound.iconSecondary : .compound.iconDisabled
)
.scaledPadding(10, relativeTo: .compound.headingLG)
case .recording:
CompoundIcon(asset: Asset.Images.stopRecording, size: .medium, relativeTo: .compound.headingLG)
Expand Down
1 change: 1 addition & 0 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenCoordinator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ final class RoomScreenCoordinator: CoordinatorProtocol {
maxExpandedHeight: ComposerConstant.maxHeight,
parserStyle: .elementX)
let composerViewModel = ComposerToolbarViewModel(initialText: parameters.sharedText,
roomProxy: parameters.roomProxy,
wysiwygViewModel: wysiwygViewModel,
completionSuggestionService: parameters.completionSuggestionService,
mediaProvider: parameters.mediaProvider,
Expand Down
2 changes: 2 additions & 0 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ struct RoomScreenViewStateBindings { }

enum RoomScreenFooterViewAction {
case resolvePinViolation(userID: String)
case resolveVerificationViolation(userID: String)
}

enum RoomScreenFooterViewDetails {
case pinViolation(member: RoomMemberProxyProtocol, learnMoreURL: URL)
case verificationViolation(member: RoomMemberProxyProtocol, learnMoreURL: URL)
}

enum PinnedEventsBannerState: Equatable {
Expand Down
Loading

0 comments on commit f20847f

Please sign in to comment.