-
-
Notifications
You must be signed in to change notification settings - Fork 279
replay: add sensitive content widget to default masking #2989
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 26 commits
333f93a
a240674
6d1e35e
74eb694
2e01b33
132f050
591b9c3
5ce2839
dfce0e8
efa9362
f76a31d
b928028
f4c8e58
151a70d
c8596cd
dde1132
7c9f696
088cf62
4bbbded
20657ad
32e3fc4
0c59077
686750f
1f8bf63
e38ab52
33b5751
87f3e2e
e8fab32
fbc6634
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import 'package:flutter/services.dart'; | ||
| import 'package:flutter/widgets.dart'; | ||
| import 'package:flutter_test/flutter_test.dart'; | ||
| import 'package:sentry_flutter/sentry_flutter.dart'; | ||
|
|
@@ -125,7 +126,7 @@ void main() async { | |
| SentryMaskingDecision.unmask); | ||
| }); | ||
|
|
||
| testWidgets('retuns false if no rule matches', (tester) async { | ||
| testWidgets('returns false if no rule matches', (tester) async { | ||
| final sut = SentryMaskingConfig([ | ||
| SentryMaskingCustomRule<Image>( | ||
| callback: (e, w) => SentryMaskingDecision.continueProcessing, | ||
|
|
@@ -169,6 +170,7 @@ void main() async { | |
| 'SentryMaskingConstantRule<Text>(mask)', | ||
| 'SentryMaskingConstantRule<EditableText>(mask)', | ||
| 'SentryMaskingConstantRule<RichText>(mask)', | ||
| ..._maybeWithSensitiveContent(), | ||
| 'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)' | ||
| ]); | ||
| }); | ||
|
|
@@ -181,6 +183,7 @@ void main() async { | |
| expect(rulesAsStrings(sut), [ | ||
| ...alwaysEnabledRules, | ||
| 'SentryMaskingConstantRule<Image>(mask)', | ||
| ..._maybeWithSensitiveContent(), | ||
| 'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)' | ||
| ]); | ||
| }); | ||
|
|
@@ -193,6 +196,7 @@ void main() async { | |
| expect(rulesAsStrings(sut), [ | ||
| ...alwaysEnabledRules, | ||
| 'SentryMaskingCustomRule<Image>(Mask all images except asset images.)', | ||
| ..._maybeWithSensitiveContent(), | ||
| 'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)' | ||
| ]); | ||
| }); | ||
|
|
@@ -207,6 +211,7 @@ void main() async { | |
| 'SentryMaskingConstantRule<Text>(mask)', | ||
| 'SentryMaskingConstantRule<EditableText>(mask)', | ||
| 'SentryMaskingConstantRule<RichText>(mask)', | ||
| ..._maybeWithSensitiveContent(), | ||
| 'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)' | ||
| ]); | ||
| }); | ||
|
|
@@ -218,31 +223,66 @@ void main() async { | |
| ..maskAssetImages = false; | ||
| expect(rulesAsStrings(sut), [ | ||
| ...alwaysEnabledRules, | ||
| ..._maybeWithSensitiveContent(), | ||
| 'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)' | ||
| ]); | ||
| }); | ||
|
|
||
| test( | ||
| 'SensitiveContent rule is automatically added when current Flutter version is equal or newer than 3.33', | ||
| () { | ||
| final sut = SentryPrivacyOptions(); | ||
| final version = FlutterVersion.version!; | ||
| final dot = version.indexOf('.'); | ||
| final major = int.tryParse(version.substring(0, dot)); | ||
| final nextDot = version.indexOf('.', dot + 1); | ||
| final minor = int.tryParse( | ||
| version.substring(dot + 1, nextDot == -1 ? version.length : nextDot)); | ||
|
|
||
| if (major! > 3 || (major == 3 && minor! >= 33)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Version Parsing Flaw in Test CodeThe version parsing logic in
buenaflor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| expect( | ||
| rulesAsStrings(sut).contains( | ||
| 'SentryMaskingCustomRule<SensitiveContent>(Mask SensitiveContent widget.)'), | ||
| isTrue, | ||
| reason: 'Test failed with version: ${FlutterVersion.version}'); | ||
| } else { | ||
| expect( | ||
| rulesAsStrings(sut).contains( | ||
| 'SentryMaskingCustomRule<SensitiveContent>(Mask SensitiveContent widget.)'), | ||
| isFalse, | ||
| reason: 'Test failed with version: ${FlutterVersion.version}'); | ||
| } | ||
| }, skip: FlutterVersion.version == null); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| group('user rules', () { | ||
| final defaultRules = [ | ||
| ...alwaysEnabledRules, | ||
| 'SentryMaskingCustomRule<Image>(Mask all images except asset images.)', | ||
| 'SentryMaskingConstantRule<Text>(mask)', | ||
| 'SentryMaskingConstantRule<EditableText>(mask)', | ||
| 'SentryMaskingConstantRule<RichText>(mask)', | ||
| ..._maybeWithSensitiveContent(), | ||
| 'SentryMaskingCustomRule<Widget>(Debug-mode-only warning for potentially sensitive widgets.)' | ||
| ]; | ||
|
|
||
| test('mask() takes precedence', () { | ||
| final sut = SentryPrivacyOptions(); | ||
| sut.mask<Image>(); | ||
| expect(rulesAsStrings(sut), | ||
| ['SentryMaskingConstantRule<Image>(mask)', ...defaultRules]); | ||
| expect(rulesAsStrings(sut), [ | ||
| 'SentryMaskingConstantRule<Image>(mask)', | ||
| ...defaultRules, | ||
| ]); | ||
| }); | ||
|
|
||
| test('unmask() takes precedence', () { | ||
| final sut = SentryPrivacyOptions(); | ||
| sut.unmask<Image>(); | ||
| expect(rulesAsStrings(sut), | ||
| ['SentryMaskingConstantRule<Image>(unmask)', ...defaultRules]); | ||
| expect(rulesAsStrings(sut), [ | ||
| 'SentryMaskingConstantRule<Image>(unmask)', | ||
| ...defaultRules, | ||
| ]); | ||
| }); | ||
|
|
||
| test('are ordered in the call order', () { | ||
| var sut = SentryPrivacyOptions(); | ||
| sut.mask<Image>(); | ||
|
|
@@ -272,13 +312,14 @@ void main() async { | |
| ...defaultRules | ||
| ]); | ||
| }); | ||
|
|
||
| test('maskCallback() takes precedence', () { | ||
| final sut = SentryPrivacyOptions(); | ||
| sut.maskCallback( | ||
| (Element element, Image widget) => SentryMaskingDecision.mask); | ||
| expect(rulesAsStrings(sut), [ | ||
| 'SentryMaskingCustomRule<Image>(Custom callback-based rule (description unspecified))', | ||
| ...defaultRules | ||
| ...defaultRules, | ||
| ]); | ||
| }); | ||
| test('User cannot add $SentryMask and $SentryUnmask rules', () { | ||
|
|
@@ -320,6 +361,25 @@ void main() async { | |
| }); | ||
| } | ||
|
|
||
| List<String> _maybeWithSensitiveContent() { | ||
| final version = FlutterVersion.version; | ||
| if (version == null) { | ||
| return []; | ||
| } | ||
| final dot = version.indexOf('.'); | ||
buenaflor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| final major = int.tryParse(version.substring(0, dot)); | ||
| final nextDot = version.indexOf('.', dot + 1); | ||
| final minor = int.tryParse( | ||
| version.substring(dot + 1, nextDot == -1 ? version.length : nextDot)); | ||
| if (major! > 3 || (major == 3 && minor! >= 33)) { | ||
| return [ | ||
| 'SentryMaskingCustomRule<SensitiveContent>(Mask SensitiveContent widget.)' | ||
| ]; | ||
| } else { | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| extension on Element { | ||
| Element findFirstOfType<T>() { | ||
| late Element result; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Could do early returns when either major/minor is null. Also, do we do version check anywhere else? Then we could extract the version reading to a common place and only do the version check here, as we do both now.