From 36d7b7887ad8bee1b839523133b0bc8ae7a5843c Mon Sep 17 00:00:00 2001 From: Maciek Grzybowski Date: Fri, 5 May 2023 16:26:17 +0200 Subject: [PATCH 1/7] REPLAY-1610 Centralise the classification of "sensitive text" --- .../Recorder/Utilities/UIKitExtensions.swift | 36 +++++++++++++++++++ .../NodeRecorders/UITextFieldRecorder.swift | 4 +-- .../NodeRecorders/UITextViewRecorder.swift | 4 +-- .../Tests/Mocks/UIKitMocks.swift | 24 +++++++++++++ .../Utilties/UIKitExtensionsTests.swift | 30 ++++++++++++++++ .../UITextFieldRecorderTests.swift | 4 +-- .../UITextViewRecorderTests.swift | 4 +-- 7 files changed, 96 insertions(+), 10 deletions(-) diff --git a/DatadogSessionReplay/Sources/Recorder/Utilities/UIKitExtensions.swift b/DatadogSessionReplay/Sources/Recorder/Utilities/UIKitExtensions.swift index 5cb1587cd0..5e78617b60 100644 --- a/DatadogSessionReplay/Sources/Recorder/Utilities/UIKitExtensions.swift +++ b/DatadogSessionReplay/Sources/Recorder/Utilities/UIKitExtensions.swift @@ -15,3 +15,39 @@ internal extension UIView { } } } + +/// Sensitive text content types as defined in Session Replay. +internal let sensitiveContentTypes: Set = { + var all: Set = [ + .password, + .emailAddress, + .telephoneNumber, + .addressCity, .addressState, .addressCityAndState, .fullStreetAddress, .streetAddressLine1, .streetAddressLine2, .postalCode, + .creditCardNumber + ] + + if #available(iOS 12.0, *) { + all.formUnion([.newPassword, .oneTimeCode]) + } + + return all +}() + +internal extension UITextInputTraits { + /// Whether or not these input traits describe a "sensitive text" as we define it in Session Replay. + /// + /// Sensitive texts include: + /// - passwords, e-mails, phone numbers, address information, credit card numbers and one-time codes; + /// - all texts marked explicitly as secure entry. + var isSensitiveText: Bool { + if isSecureTextEntry == true { + return true + } + + if let contentType = textContentType, let contentType = contentType { + return sensitiveContentTypes.contains(contentType) + } + + return false + } +} diff --git a/DatadogSessionReplay/Sources/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextFieldRecorder.swift b/DatadogSessionReplay/Sources/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextFieldRecorder.swift index 57937c9215..b136c92f1d 100644 --- a/DatadogSessionReplay/Sources/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextFieldRecorder.swift +++ b/DatadogSessionReplay/Sources/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextFieldRecorder.swift @@ -83,8 +83,6 @@ internal struct UITextFieldRecorder: NodeRecorder { let textFrame = attributes.frame .insetBy(dx: 5, dy: 5) // 5 points padding - let isSensitiveText = textField.isSecureTextEntry || textField.textContentType == .emailAddress || textField.textContentType == .telephoneNumber - let builder = UITextFieldWireframesBuilder( wireframeRect: textFrame, attributes: attributes, @@ -95,7 +93,7 @@ internal struct UITextFieldRecorder: NodeRecorder { isPlaceholderText: isPlaceholder, font: textField.font, fontScalingEnabled: textField.adjustsFontSizeToFitWidth, - textObfuscator: textObfuscator(context, isSensitiveText) + textObfuscator: textObfuscator(context, textField.isSensitiveText) ) return Node(viewAttributes: attributes, wireframesBuilder: builder) } diff --git a/DatadogSessionReplay/Sources/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextViewRecorder.swift b/DatadogSessionReplay/Sources/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextViewRecorder.swift index a82f15668d..085c8b1bcb 100644 --- a/DatadogSessionReplay/Sources/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextViewRecorder.swift +++ b/DatadogSessionReplay/Sources/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextViewRecorder.swift @@ -27,8 +27,6 @@ internal struct UITextViewRecorder: NodeRecorder { return InvisibleElement.constant } - let isSensitiveText = textView.isSecureTextEntry || textView.textContentType == .emailAddress || textView.textContentType == .telephoneNumber - let builder = UITextViewWireframesBuilder( wireframeID: context.ids.nodeID(for: textView), attributes: attributes, @@ -36,7 +34,7 @@ internal struct UITextViewRecorder: NodeRecorder { textAlignment: textView.textAlignment, textColor: textView.textColor?.cgColor ?? UIColor.black.cgColor, font: textView.font, - textObfuscator: textObfuscator(context, isSensitiveText), + textObfuscator: textObfuscator(context, textView.isSensitiveText), contentRect: CGRect(origin: textView.contentOffset, size: textView.contentSize) ) let node = Node(viewAttributes: attributes, wireframesBuilder: builder) diff --git a/DatadogSessionReplay/Tests/Mocks/UIKitMocks.swift b/DatadogSessionReplay/Tests/Mocks/UIKitMocks.swift index 1bb54596d8..1b784909cc 100644 --- a/DatadogSessionReplay/Tests/Mocks/UIKitMocks.swift +++ b/DatadogSessionReplay/Tests/Mocks/UIKitMocks.swift @@ -81,3 +81,27 @@ extension UIView.ContentMode { UIView.ContentMode(rawValue: Int.random(in: 0...12)) ?? .scaleToFill } } + +extension UITextContentType: RandomMockable { + static var allCases: Set { + var all: Set = [ + .name, .namePrefix, .givenName, .middleName, .familyName, .nameSuffix, .nickname, + .jobTitle, .organizationName, .location, .fullStreetAddress, .streetAddressLine1, + .streetAddressLine2, .addressCity, .addressState, .addressCityAndState, .sublocality, + .countryName, .postalCode, .telephoneNumber, .emailAddress, .URL, .creditCardNumber, + .username, .password + ] + + if #available(iOS 15.0, *) { + all.formUnion([.shipmentTrackingNumber, .flightNumber, .dateTime]) + } + + if #available(iOS 12.0, *) { + all.formUnion([.newPassword, .oneTimeCode]) + } + + return all + } + + public static func mockRandom() -> UITextContentType { allCases.randomElement()! } +} diff --git a/DatadogSessionReplay/Tests/Recorder/Utilties/UIKitExtensionsTests.swift b/DatadogSessionReplay/Tests/Recorder/Utilties/UIKitExtensionsTests.swift index dc9ac76a6f..f9b6af7d48 100644 --- a/DatadogSessionReplay/Tests/Recorder/Utilties/UIKitExtensionsTests.swift +++ b/DatadogSessionReplay/Tests/Recorder/Utilties/UIKitExtensionsTests.swift @@ -7,6 +7,7 @@ import XCTest import UIKit @testable import DatadogSessionReplay +@testable import TestUtilities class UIKitExtensionsTests: XCTestCase { func testUsesDarkMode() { @@ -27,4 +28,33 @@ class UIKitExtensionsTests: XCTestCase { XCTAssertFalse(darkView.usesDarkMode) } } + + // swiftlint:disable opening_brace + func testIsSensitiveText() { + class Mock: NSObject, UITextInputTraits { + var isSecureTextEntry = false + var textContentType: UITextContentType! = nil // swiftlint:disable:this implicitly_unwrapped_optional + } + + // Given + let sensitiveTextMock = Mock() + let nonSensitiveTextMock = Mock() + let nonSensitiveContentTypes = UITextContentType.allCases.subtracting(sensitiveContentTypes) + + // When + oneOrMoreOf([ + { sensitiveTextMock.isSecureTextEntry = true }, + { sensitiveTextMock.textContentType = sensitiveContentTypes.randomElement() }, + ]) + oneOrMoreOf([ + { nonSensitiveTextMock.isSecureTextEntry = false }, + { nonSensitiveTextMock.textContentType = nil }, + { nonSensitiveTextMock.textContentType = nonSensitiveContentTypes.randomElement() }, + ]) + + // Then + XCTAssertTrue(sensitiveTextMock.isSensitiveText) + XCTAssertFalse(nonSensitiveTextMock.isSensitiveText) + } + // swiftlint:enable opening_brace } diff --git a/DatadogSessionReplay/Tests/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextFieldRecorderTests.swift b/DatadogSessionReplay/Tests/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextFieldRecorderTests.swift index 2375b9b240..048d213698 100644 --- a/DatadogSessionReplay/Tests/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextFieldRecorderTests.swift +++ b/DatadogSessionReplay/Tests/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextFieldRecorderTests.swift @@ -84,9 +84,9 @@ class UITextFieldRecorderTests: XCTestCase { XCTAssertTrue(try textObfuscator(in: .maskUserInput) is FixLegthMaskObfuscator) // When - oneOf([ + oneOrMoreOf([ { self.textField.isSecureTextEntry = true }, - { self.textField.textContentType = [.telephoneNumber, .emailAddress].randomElement()! }, + { self.textField.textContentType = sensitiveContentTypes.randomElement() }, ]) // Then diff --git a/DatadogSessionReplay/Tests/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextViewRecorderTests.swift b/DatadogSessionReplay/Tests/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextViewRecorderTests.swift index 9c5b4ec185..519684ab98 100644 --- a/DatadogSessionReplay/Tests/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextViewRecorderTests.swift +++ b/DatadogSessionReplay/Tests/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/UITextViewRecorderTests.swift @@ -74,9 +74,9 @@ class UITextViewRecorderTests: XCTestCase { XCTAssertTrue(try textObfuscator(in: .maskUserInput) is SpacePreservingMaskObfuscator) // When - oneOf([ + oneOrMoreOf([ { self.textView.isSecureTextEntry = true }, - { self.textView.textContentType = [.telephoneNumber, .emailAddress].randomElement()! }, + { self.textView.textContentType = sensitiveContentTypes.randomElement() }, ]) // Then From ec294a66eafa847dc907779128d5c459bef976bb Mon Sep 17 00:00:00 2001 From: Maciek Grzybowski Date: Fri, 5 May 2023 16:35:33 +0200 Subject: [PATCH 2/7] REPLAY-1610 Change the falue of fix-length mask --- .../Sources/Processor/Privacy/TextObfuscator.swift | 6 +++--- .../Tests/Processor/Privacy/TextObfuscatorTests.swift | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/DatadogSessionReplay/Sources/Processor/Privacy/TextObfuscator.swift b/DatadogSessionReplay/Sources/Processor/Privacy/TextObfuscator.swift index 879c0de82c..794e62e681 100644 --- a/DatadogSessionReplay/Sources/Processor/Privacy/TextObfuscator.swift +++ b/DatadogSessionReplay/Sources/Processor/Privacy/TextObfuscator.swift @@ -19,7 +19,7 @@ internal struct TextObfuscators { let nop = NOPTextObfuscator() /// Text obfuscator that replaces each character with `"x"` mask. let spacePreservingMask = SpacePreservingMaskObfuscator() - /// Text obfuscator that replaces whole text with fixed-length `"xxx"` mask (three asterics). + /// Text obfuscator that replaces whole text with fixed-length `"***"` mask (three asterics). let fixLegthMask = FixLegthMaskObfuscator() } @@ -49,9 +49,9 @@ internal struct SpacePreservingMaskObfuscator: TextObfuscating { } } -/// Text obfuscator which replaces entire text with fix-length `"xxx"` mask value. +/// Text obfuscator which replaces entire text with fix-length `"***"` mask value. internal struct FixLegthMaskObfuscator: TextObfuscating { - private static let maskedString = "xxx" + private static let maskedString = "***" func mask(text: String) -> String { Self.maskedString } } diff --git a/DatadogSessionReplay/Tests/Processor/Privacy/TextObfuscatorTests.swift b/DatadogSessionReplay/Tests/Processor/Privacy/TextObfuscatorTests.swift index ff2700ee6c..b9135ea61d 100644 --- a/DatadogSessionReplay/Tests/Processor/Privacy/TextObfuscatorTests.swift +++ b/DatadogSessionReplay/Tests/Processor/Privacy/TextObfuscatorTests.swift @@ -54,7 +54,7 @@ class FixLegthMaskObfuscatorTests: XCTestCase { let obfuscator = FixLegthMaskObfuscator() func testWhenObfuscatingItAlwaysReplacesTextItWithConstantMask() { - let expectedMask = "xxx" + let expectedMask = "***" XCTAssertEqual(obfuscator.mask(text: .mockRandom(among: .alphanumericsAndWhitespace)), expectedMask) XCTAssertEqual(obfuscator.mask(text: .mockRandom(among: .allUnicodes)), expectedMask) From 71677703020d230d274c019835e66deecfe0bc07 Mon Sep 17 00:00:00 2001 From: Maciek Grzybowski Date: Fri, 5 May 2023 17:02:48 +0200 Subject: [PATCH 3/7] REPLAY-1610 Align rules of text masking --- .../SRHost/Fixtures/Basic.storyboard | 24 ++++++++++++++----- .../NodeRecorders/UISegmentRecorder.swift | 4 ++-- .../NodeRecorders/UITextFieldRecorder.swift | 24 ++++++++++++------- .../NodeRecorders/UITextViewRecorder.swift | 22 +++++++++++------ .../UITextFieldRecorderTests.swift | 11 ++++++++- .../UITextViewRecorderTests.swift | 15 ++++++++++-- 6 files changed, 73 insertions(+), 27 deletions(-) diff --git a/DatadogSessionReplay/SRSnapshotTests/SRHost/Fixtures/Basic.storyboard b/DatadogSessionReplay/SRSnapshotTests/SRHost/Fixtures/Basic.storyboard index 49b2efd5dc..d46defffe2 100644 --- a/DatadogSessionReplay/SRSnapshotTests/SRHost/Fixtures/Basic.storyboard +++ b/DatadogSessionReplay/SRSnapshotTests/SRHost/Fixtures/Basic.storyboard @@ -78,9 +78,9 @@ - + -