REPLAY-1610 Align text and appearance masking rules

[ncreated]

What and why?

📦 This PR aligns rules of text and appearance masking to what was concluded in Privacy Options in Mobile SR:

How?

For text masking, above configuration is reflected in SessionReplayPrivacy extension:

/// Text masking strategies per type of text.
extension SessionReplayPrivacy {
    var sensitiveTextObfuscator: TextObfuscating
    var inputAndOptionTextObfuscator: TextObfuscating
    var staticTextObfuscator: TextObfuscating
    var hintTextObfuscator: TextObfuscating
}

Each node recorder uses TextObfuscation to obtain strategy respectively to its text type (e.g. label recorder uses staticTextObfuscator(for:)). Same as before, compound recorders can configure child recorders with alternative strategies (e.g. label recorder in picker view recorder uses inputAndOptionTextObfuscator(for:)).

For appearance masking, we had to update the UISliderRecorder (other recorders were fine) so it erases selected value in .maskAll and .maskUserInput modes:

`.allowAll`	`.maskAll`	`.maskUserInput`

No changes to touch interaction masking - it's already good.

---

This PR alters most of the snapshot files, but due to their number only few representative examples:

`.allowAll`	`.maskAll`	`.maskUserInput`

Review checklist

• Feature or bugfix MUST have appropriate tests (unit, integration)
• Make sure each commit and the PR mention the Issue number or JIRA reference
• Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

• Run unit tests
• Run integration tests
• Run smoke tests

[ncreated]

💡 Passing context.*.privacy to context.textObfuscation.*() might look odd for a moment, but it will get more sense once per-element masking is available later in fine grained privacy project:

context.textObfuscation.staticTextObfuscator(for: overridenPrivacy ?? context.recorder.privacy)

[maxep]

To me, TextObfuscation as an object is kind of superfluous: it's not mockable and makes the call stack a bit odd as you mentioned. I think it would make more sense as a SessionReplayPrivacy extension:

// TextObfuscator.swift

extension SessionReplayPrivacy {
    /// Returns "Static Text" obfuscator for this privacy level.
    ///
    /// In Session Replay, "Static Text" is a text not directly entered by the user.
    var staticTextObfuscator: TextObfuscating {
        switch self {
        case .allowAll:         return NOPTextObfuscator()
        case .maskAll:          return SpacePreservingMaskObfuscator()
        case .maskUserInput:    return NOPTextObfuscator()
        }
    }
}

Then this closure would simply read:

(overridenPrivacy ?? context.recorder.privacy).staticTextObfuscator

WDYT?

[ganeshnj]

I think, we don't have a right abstraction here. I kind of liked what Max proposed but with slightly protocol oriented.

protocol TextObfuscator {
    func obfuscate(for privacyLevel: SessionReplayPrivacy) -> TextObfuscating
}

struct sensitiveTextObfuscator: TextObfuscator {
    func obfuscate(for privacyLevel: SessionReplayPrivacy) -> TextObfuscating {
        ...
    }
}

Usage

context.textObfuscator.obfuscate(for: context.recorder.privacy)

[ncreated]

@maxep there is the performance characteristics we anticipate in SpacePreservingMaskObfuscator that could eventually make this idea slightly less convenient. It works O(n) today, but O(1) is possible with LRU-like caching. It runs on background thread, so it's not that much of a problem, but anticipating apps with long texts (e.g. social media posts), it could make sense to cache it. If adding caching, it the context.textObfuscator would become truer dependency injection mechanism.

However, thinking twice I tend to realise that it might be pre-mature. Adding cache to SPMObfuscator is not impossible with your proposal, so I updated code with what you proposed. I agree that it results with more consistent domain definition 👌.

---

@ganeshnj if I get the idea right, this won't be correct to our problematic:

context.textObfuscator.obfuscate(for: context.recorder.privacy)

Some cases are more complex as this, e.g. text field masking is currently:

if isPlaceholder {
    return context.recorder.privacy.hintTextObfuscator
} else if isSensitive {
    return context.recorder.privacy.sensitiveTextObfuscator
} else {
    return context.recorder.privacy.inputAndOptionTextObfuscator
}

so it can't use single context.textObfuscator with different implementations. There is also a challenge of elements that need to be masked differently in different subtrees, described in more details in #1237 (2nd point in "How?").

I think we're "good enough" with @maxep 's proposal.

[ganeshnj]

for my understanding

are isPlaceholder, isSensitive set before the obfuscator is decided or later?

[ncreated]

They are set before (we infer both from UITextField instance). Based on that, we decide if the text should be classified as "Hint Text", "Sensitive Text" or "Input & Option Text" according to Privacy Options in Mobile SR. The category of the text and current privacy are then used to decide on which obfuscator implementation will be used: NOP, FixLength or SpacePreserving.

[ncreated]

💡 Here, I'm enhancing the BasicTexts fixture. Now it shows TextView in editable and non-editable state, as both are masked differently (respectively as "Input & Option Text" and "Static Text"), e.g.:

`.allowAll`	`.maskUserInput`

[maciejburda]

LGTM! 🎉

[maxep]

Looks great!

I've left a suggestion regarding TextObfuscation which could be replaced by a SessionReplayPrivacy extension. It would fit better in the recorders design. LMKWYT!

[maxep]

To me, TextObfuscation as an object is kind of superfluous: it's not mockable and makes the call stack a bit odd as you mentioned. I think it would make more sense as a SessionReplayPrivacy extension:

// TextObfuscator.swift

extension SessionReplayPrivacy {
    /// Returns "Static Text" obfuscator for this privacy level.
    ///
    /// In Session Replay, "Static Text" is a text not directly entered by the user.
    var staticTextObfuscator: TextObfuscating {
        switch self {
        case .allowAll:         return NOPTextObfuscator()
        case .maskAll:          return SpacePreservingMaskObfuscator()
        case .maskUserInput:    return NOPTextObfuscator()
        }
    }
}

Then this closure would simply read:

(overridenPrivacy ?? context.recorder.privacy).staticTextObfuscator

WDYT?

[maxep]

TextObfuscation is not an abstraction so we don't get much value of injecting it, using an SessionReplayPrivacy extension as mentioned would simplify things.

[ncreated]

Agree - although as explained, I was anticipating adding cache later. That way, the VTSBuilder would orchestrate it. Anyway, this was revamped according to your proposal ✅

[maxep]

Nice work @ncreated ! and kudos for the great description and comments 🙏