From a5d4d1c93c0f2561e7150535a796709b6db9ed9b Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 5 Apr 2024 09:53:07 +0200 Subject: [PATCH 1/2] Telemetry | Add more properties to posthog utd errors --- .../DecryptionFailure+Analytics.swift | 7 +- .../Modules/Analytics/DecryptionFailure.swift | 8 ++ .../Analytics/DecryptionFailureTracker.swift | 11 +++ RiotTests/DecryptionFailureTrackerTests.swift | 91 ++++++++++++++++--- 4 files changed, 102 insertions(+), 15 deletions(-) diff --git a/Riot/Modules/Analytics/DecryptionFailure+Analytics.swift b/Riot/Modules/Analytics/DecryptionFailure+Analytics.swift index 5ed3c36c2c..119366a665 100644 --- a/Riot/Modules/Analytics/DecryptionFailure+Analytics.swift +++ b/Riot/Modules/Analytics/DecryptionFailure+Analytics.swift @@ -42,14 +42,13 @@ extension DecryptionFailure { cryptoModule: .Rust, cryptoSDK: .Rust, domain: .E2EE, - eventLocalAgeMillis: self.eventLocalAgeMillis, - isFederated: nil, - isMatrixDotOrg: nil, + isFederated: self.isFederated, + isMatrixDotOrg: self.isMatrixOrg, name: errorName, timeToDecryptMillis: timeToDecryptMillis, userTrustsOwnIdentity: self.trustOwnIdentityAtTimeOfFailure, - wasVisibleToUser: nil + wasVisibleToUser: self.wasVisibleToUser ) } } diff --git a/Riot/Modules/Analytics/DecryptionFailure.swift b/Riot/Modules/Analytics/DecryptionFailure.swift index 6d3f6c1a83..179509087e 100644 --- a/Riot/Modules/Analytics/DecryptionFailure.swift +++ b/Riot/Modules/Analytics/DecryptionFailure.swift @@ -52,6 +52,14 @@ import AnalyticsEvents var eventLocalAgeMillis: Int? + /// Is the current user on matrix org + var isMatrixOrg: Bool? + /// Are the sender and recipient on the same homeserver + var isFederated: Bool? + + /// As for now the ios App only reports UTDs visible to user (error are reported from EventFormatter + var wasVisibleToUser: Bool = true + init(failedEventId: String, reason: DecryptionFailureReason, context: String, ts: TimeInterval) { self.failedEventId = failedEventId self.reason = reason diff --git a/Riot/Modules/Analytics/DecryptionFailureTracker.swift b/Riot/Modules/Analytics/DecryptionFailureTracker.swift index 503c733ba3..0832476ffe 100644 --- a/Riot/Modules/Analytics/DecryptionFailureTracker.swift +++ b/Riot/Modules/Analytics/DecryptionFailureTracker.swift @@ -103,6 +103,17 @@ class DecryptionFailureTracker: NSObject { failure.eventLocalAgeMillis = Int(exactly: eventRelativeAgeMillis) failure.trustOwnIdentityAtTimeOfFailure = isSessionVerified + let myDomain = userId.components(separatedBy: ":")[1] + failure.isMatrixOrg = myDomain == "matrix.org" + + if MXTools.isMatrixUserIdentifier(event.sender) { + let senderDomain = event.sender.components(separatedBy: ":")[1] + failure.isFederated = senderDomain != myDomain + } + + /// XXX for future work, as for now only the event formatter reports UTDs. That means that it's only UTD ~visible to users + failure.wasVisibleToUser = true + reportedFailures[failedEventId] = failure diff --git a/RiotTests/DecryptionFailureTrackerTests.swift b/RiotTests/DecryptionFailureTrackerTests.swift index c142514cb0..d37b96f081 100644 --- a/RiotTests/DecryptionFailureTrackerTests.swift +++ b/RiotTests/DecryptionFailureTrackerTests.swift @@ -56,7 +56,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_grace_period() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let decryptionFailureTracker = DecryptionFailureTracker(); @@ -95,7 +95,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_report_ratcheted_key_utd() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let decryptionFailureTracker = DecryptionFailureTracker(); @@ -125,7 +125,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_report_unspecified_error() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let decryptionFailureTracker = DecryptionFailureTracker(); @@ -157,7 +157,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_do_not_double_report() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let decryptionFailureTracker = DecryptionFailureTracker(); @@ -199,7 +199,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_ignore_not_member() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let decryptionFailureTracker = DecryptionFailureTracker(); @@ -234,7 +234,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_notification_center() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let decryptionFailureTracker = DecryptionFailureTracker(); @@ -275,7 +275,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_should_report_late_decrypt() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let decryptionFailureTracker = DecryptionFailureTracker(); @@ -320,7 +320,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_should_report_permanent_decryption_error() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let decryptionFailureTracker = DecryptionFailureTracker(); @@ -361,7 +361,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_should_report_trust_status_at_decryption_time() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let decryptionFailureTracker = DecryptionFailureTracker(); @@ -425,7 +425,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_should_report_event_age() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let format = DateFormatter() @@ -477,7 +477,7 @@ class DecryptionFailureTrackerTests: XCTestCase { func test_should_report_expected_utds() { - let myUser = "test@example.com"; + let myUser = "@test:example.com"; fakeSession.mockUserId = myUser; let format = DateFormatter() @@ -529,5 +529,74 @@ class DecryptionFailureTrackerTests: XCTestCase { } + + func test_should_report_is_matrix_org_and_is_federated() { + + let myUser = "@test:example.com"; + fakeSession.mockUserId = myUser; + + let decryptionFailureTracker = DecryptionFailureTracker(); + decryptionFailureTracker.timeProvider = timeShifter; + + let testDelegate = AnalyticsDelegate(); + + decryptionFailureTracker.delegate = testDelegate; + + timeShifter.timestamp = TimeInterval(0) + + let fakeEvent = FakeEvent(id: "$0000"); + fakeEvent.sender = "@bob:example.com" + fakeEvent.decryptionError = NSError(domain: MXDecryptingErrorDomain, code: Int(MXDecryptingErrorUnknownInboundSessionIdCode.rawValue)) + + // set session as not yet verified + fakeCrossSigning.canTrustCrossSigning = false + + let fakeRoomState = FakeRoomState(); + fakeRoomState.mockMembers = FakeRoomMembers(joined: [myUser]) + + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent, withRoomState: fakeRoomState, mySession: fakeSession); + + // Simulate succesful decryption after max wait + timeShifter.timestamp = TimeInterval(70) + + decryptionFailureTracker.checkFailures(); + + XCTAssertEqual(testDelegate.reportedFailure?.isMatrixOrg, false); + XCTAssertEqual(testDelegate.reportedFailure?.isFederated, false); + + + let analyticsError = testDelegate.reportedFailure!.toAnalyticsEvent() + + XCTAssertEqual(analyticsError.isMatrixDotOrg, false) + XCTAssertEqual(analyticsError.isFederated, false) + + // Report a new error now that session is verified + + let fakeEvent2 = FakeEvent(id: "$0001"); + fakeEvent2.sender = "@bob:example.com" + fakeEvent2.decryptionError = NSError(domain: MXDecryptingErrorDomain, code: Int(MXDecryptingErrorUnknownInboundSessionIdCode.rawValue)) + + fakeSession.mockUserId = "@test:matrix.org"; + fakeRoomState.mockMembers = FakeRoomMembers(joined: [fakeSession.mockUserId]) + + decryptionFailureTracker.reportUnableToDecryptError(forEvent: fakeEvent2, withRoomState: fakeRoomState, mySession: fakeSession); + + // Simulate permanent UTD + timeShifter.timestamp = TimeInterval(140) + + decryptionFailureTracker.checkFailures(); + + XCTAssertEqual(testDelegate.reportedFailure?.failedEventId, "$0001"); + XCTAssertEqual(testDelegate.reportedFailure?.isMatrixOrg, true); + XCTAssertEqual(testDelegate.reportedFailure?.isFederated, true); + + let analyticsError2 = testDelegate.reportedFailure!.toAnalyticsEvent() + + XCTAssertEqual(analyticsError2.isMatrixDotOrg, true) + XCTAssertEqual(analyticsError2.isFederated, true) + + } + + } From afd6deea86b052e933f2e7f6c13cb79f3bd59cd8 Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 15 Apr 2024 10:00:19 +0200 Subject: [PATCH 2/2] review use array last for safety --- Riot/Modules/Analytics/DecryptionFailureTracker.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Riot/Modules/Analytics/DecryptionFailureTracker.swift b/Riot/Modules/Analytics/DecryptionFailureTracker.swift index 0832476ffe..d76a38ca59 100644 --- a/Riot/Modules/Analytics/DecryptionFailureTracker.swift +++ b/Riot/Modules/Analytics/DecryptionFailureTracker.swift @@ -103,12 +103,12 @@ class DecryptionFailureTracker: NSObject { failure.eventLocalAgeMillis = Int(exactly: eventRelativeAgeMillis) failure.trustOwnIdentityAtTimeOfFailure = isSessionVerified - let myDomain = userId.components(separatedBy: ":")[1] + let myDomain = userId.components(separatedBy: ":").last failure.isMatrixOrg = myDomain == "matrix.org" if MXTools.isMatrixUserIdentifier(event.sender) { - let senderDomain = event.sender.components(separatedBy: ":")[1] - failure.isFederated = senderDomain != myDomain + let senderDomain = event.sender.components(separatedBy: ":").last + failure.isFederated = senderDomain != nil && senderDomain != myDomain } /// XXX for future work, as for now only the event formatter reports UTDs. That means that it's only UTD ~visible to users