Skip to content

feat: support masking screen names #500

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

Merged
merged 12 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## [Unreleased](https://github.com/Instabug/Instabug-Flutter/compare/v13.3.0...dev)

### Added

- Add support for masking screen names captured by Instabug through the `Instabug.setScreenNameMaskingCallback` API ([#500](https://github.com/Instabug/Instabug-Flutter/pull/500)).

## [13.3.0](https://github.com/Instabug/Instabug-Flutter/compare/v13.2.0...v13.3.0) (August 5, 2024)

### Added
Expand Down
1 change: 1 addition & 0 deletions lib/instabug_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ export 'src/modules/surveys.dart';
export 'src/utils/instabug_navigator_observer.dart';
export 'src/utils/screen_loading/instabug_capture_screen_loading.dart';
export 'src/utils/screen_loading/route_matcher.dart';
export 'src/utils/screen_name_masker.dart' show ScreenNameMaskingCallback;
11 changes: 11 additions & 0 deletions lib/src/models/instabug_route.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import 'package:flutter/material.dart';

class InstabugRoute {
final Route<dynamic> route;
final String name;

const InstabugRoute({
required this.route,
required this.name,
});
}
9 changes: 9 additions & 0 deletions lib/src/modules/instabug.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import 'package:instabug_flutter/src/generated/instabug.api.g.dart';
import 'package:instabug_flutter/src/utils/enum_converter.dart';
import 'package:instabug_flutter/src/utils/ibg_build_info.dart';
import 'package:instabug_flutter/src/utils/instabug_logger.dart';
import 'package:instabug_flutter/src/utils/screen_name_masker.dart';
import 'package:meta/meta.dart';

enum InvocationEvent {
Expand Down Expand Up @@ -191,6 +192,14 @@ class Instabug {
);
}

/// Sets a [callback] to be called wehenever a screen name is captured to mask
/// sensitive information in the screen name.
static void setScreenNameMaskingCallback(
ScreenNameMaskingCallback? callback,
) {
ScreenNameMasker.I.setMaskingCallback(callback);
}

/// Shows the welcome message in a specific mode.
/// [welcomeMessageMode] is an enum to set the welcome message mode to live, or beta.
static Future<void> showWelcomeMessageWithMode(
Expand Down
28 changes: 18 additions & 10 deletions lib/src/utils/instabug_navigator_observer.dart
Original file line number Diff line number Diff line change
@@ -1,32 +1,40 @@
import 'package:flutter/material.dart';
import 'package:instabug_flutter/instabug_flutter.dart';
import 'package:instabug_flutter/src/models/instabug_route.dart';
import 'package:instabug_flutter/src/modules/instabug.dart';
import 'package:instabug_flutter/src/utils/instabug_logger.dart';
import 'package:instabug_flutter/src/utils/screen_loading/screen_loading_manager.dart';
import 'package:instabug_flutter/src/utils/screen_name_masker.dart';

class InstabugNavigatorObserver extends NavigatorObserver {
final List<Route> _steps = <Route>[];
final List<InstabugRoute> _steps = [];

void screenChanged(Route newRoute) {
try {
final screenName = newRoute.settings.name.toString();
final maskedScreenName = ScreenNameMasker.I.mask(screenName);

final route = InstabugRoute(
route: newRoute,
name: maskedScreenName,
);

// Starts a the new UI trace which is exclusive to screen loading
ScreenLoadingManager.I.startUiTrace(screenName);
ScreenLoadingManager.I.startUiTrace(maskedScreenName, screenName);
// If there is a step that hasn't been pushed yet
if (_steps.isNotEmpty) {
// Report the last step and remove it from the list
Instabug.reportScreenChange(
_steps[_steps.length - 1].settings.name.toString(),
);
_steps.remove(_steps[_steps.length - 1]);
Instabug.reportScreenChange(_steps.last.name);
_steps.removeLast();
}

// Add the new step to the list
_steps.add(newRoute);
_steps.add(route);
Future<dynamic>.delayed(const Duration(milliseconds: 1000), () {
// If this route is in the array, report it and remove it from the list
if (_steps.contains(newRoute)) {
Instabug.reportScreenChange(screenName);
_steps.remove(newRoute);
if (_steps.contains(route)) {
Instabug.reportScreenChange(route.name);
_steps.remove(route);
}
});
} catch (e) {
Expand Down
26 changes: 20 additions & 6 deletions lib/src/utils/screen_loading/screen_loading_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,16 @@ class ScreenLoadingManager {
return sanitizedScreenName;
}

/// Starts a new UI trace with [screenName] as the public screen name and
/// [matchingScreenName] as the screen name used for matching the UI trace
/// with a Screen Loading trace.
@internal
Future<void> startUiTrace(String screenName) async {
Future<void> startUiTrace(
String screenName, [
String? matchingScreenName,
]) async {
matchingScreenName ??= screenName;

try {
resetDidStartScreenLoading();

Expand All @@ -151,10 +159,19 @@ class ScreenLoadingManager {
}

final sanitizedScreenName = sanitizeScreenName(screenName);
final sanitizedMatchingScreenName =
sanitizeScreenName(matchingScreenName);

final microTimeStamp = IBGDateTime.I.now().microsecondsSinceEpoch;
final uiTraceId = IBGDateTime.I.now().millisecondsSinceEpoch;

APM.startCpUiTrace(sanitizedScreenName, microTimeStamp, uiTraceId);
currentUiTrace = UiTrace(sanitizedScreenName, traceId: uiTraceId);

currentUiTrace = UiTrace(
screenName: sanitizedScreenName,
matchingScreenName: sanitizedMatchingScreenName,
traceId: uiTraceId,
);
} catch (error, stackTrace) {
_logExceptionErrorAndStackTrace(error, stackTrace);
}
Expand Down Expand Up @@ -183,10 +200,7 @@ class ScreenLoadingManager {
return;
}

final isSameScreen = RouteMatcher.I.match(
routePath: trace.screenName,
actualPath: currentUiTrace?.screenName,
);
final isSameScreen = currentUiTrace?.matches(trace.screenName) == true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced the direct RouteMatcher call with UiTrace.matches as the matching screen name is not exposed on the UiTrace object to avoid misuse.


final didStartLoading = currentUiTrace?.didStartScreenLoading == true;

Expand Down
28 changes: 24 additions & 4 deletions lib/src/utils/screen_loading/ui_trace.dart
Original file line number Diff line number Diff line change
@@ -1,25 +1,45 @@
import 'package:instabug_flutter/src/utils/screen_loading/route_matcher.dart';

class UiTrace {
final String screenName;

/// The screen name used while matching the UI trace with a Screen Loading
/// trace.
///
/// For example, this is set to the original screen name before masking when
/// screen names masking is enabled.
final String _matchingScreenName;

final int traceId;
bool didStartScreenLoading = false;
bool didReportScreenLoading = false;
bool didExtendScreenLoading = false;

UiTrace(
this.screenName, {
UiTrace({
required this.screenName,
required this.traceId,
});
String? matchingScreenName,
}) : _matchingScreenName = matchingScreenName ?? screenName;

UiTrace copyWith({
String? screenName,
String? matchingScreenName,
int? traceId,
}) {
return UiTrace(
screenName ?? this.screenName,
screenName: screenName ?? this.screenName,
matchingScreenName: matchingScreenName ?? _matchingScreenName,
traceId: traceId ?? this.traceId,
);
}

bool matches(String routePath) {
return RouteMatcher.I.match(
routePath: routePath,
actualPath: _matchingScreenName,
);
}

@override
String toString() {
return 'UiTrace{screenName: $screenName, traceId: $traceId, isFirstScreenLoadingReported: $didReportScreenLoading, isFirstScreenLoading: $didStartScreenLoading}';
Expand Down
45 changes: 45 additions & 0 deletions lib/src/utils/screen_name_masker.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import 'package:flutter/material.dart';

typedef ScreenNameMaskingCallback = String Function(String screen);

/// Mockable [ScreenNameMasker] responsible for masking screen names
/// before they are sent to the native SDKs.
class ScreenNameMasker {
ScreenNameMasker._();

static ScreenNameMasker _instance = ScreenNameMasker._();

static ScreenNameMasker get instance => _instance;

/// Shorthand for [instance]
static ScreenNameMasker get I => instance;

static const emptyScreenNameFallback = "N/A";

ScreenNameMaskingCallback? _screenNameMaskingCallback;

@visibleForTesting
// ignore: use_setters_to_change_properties
static void setInstance(ScreenNameMasker instance) {
_instance = instance;
}

// ignore: use_setters_to_change_properties
void setMaskingCallback(ScreenNameMaskingCallback? callback) {
_screenNameMaskingCallback = callback;
}

String mask(String screen) {
if (_screenNameMaskingCallback == null) {
return screen;
}

final maskedScreen = _screenNameMaskingCallback!(screen).trim();

if (maskedScreen.isEmpty) {
return emptyScreenNameFallback;
}

return maskedScreen;
}
}
14 changes: 14 additions & 0 deletions test/instabug_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:instabug_flutter/instabug_flutter.dart';
import 'package:instabug_flutter/src/generated/instabug.api.g.dart';
import 'package:instabug_flutter/src/utils/enum_converter.dart';
import 'package:instabug_flutter/src/utils/ibg_build_info.dart';
import 'package:instabug_flutter/src/utils/screen_name_masker.dart';
import 'package:mockito/annotations.dart';
import 'package:mockito/mockito.dart';

Expand All @@ -14,17 +15,20 @@ import 'instabug_test.mocks.dart';
@GenerateMocks([
InstabugHostApi,
IBGBuildInfo,
ScreenNameMasker,
])
void main() {
TestWidgetsFlutterBinding.ensureInitialized();
WidgetsFlutterBinding.ensureInitialized();

final mHost = MockInstabugHostApi();
final mBuildInfo = MockIBGBuildInfo();
final mScreenNameMasker = MockScreenNameMasker();

setUpAll(() {
Instabug.$setHostApi(mHost);
IBGBuildInfo.setInstance(mBuildInfo);
ScreenNameMasker.setInstance(mScreenNameMasker);
});

test('[setEnabled] should call host method', () async {
Expand Down Expand Up @@ -76,6 +80,16 @@ void main() {
).called(1);
});

test(
'[setScreenNameMaskingCallback] should set masking callback on screen name masker',
() async {
String callback(String screen) => 'REDACTED/$screen';

Instabug.setScreenNameMaskingCallback(callback);

verify(mScreenNameMasker.setMaskingCallback(callback)).called(1);
});

test('[show] should call host method', () async {
await Instabug.show();

Expand Down
29 changes: 26 additions & 3 deletions test/utils/instabug_navigator_observer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'package:flutter_test/flutter_test.dart';
import 'package:instabug_flutter/instabug_flutter.dart';
import 'package:instabug_flutter/src/generated/instabug.api.g.dart';
import 'package:instabug_flutter/src/utils/screen_loading/screen_loading_manager.dart';
import 'package:instabug_flutter/src/utils/screen_name_masker.dart';
import 'package:mockito/annotations.dart';
import 'package:mockito/mockito.dart';

Expand Down Expand Up @@ -35,6 +36,8 @@ void main() {
observer = InstabugNavigatorObserver();
route = createRoute(screen);
previousRoute = createRoute(previousScreen);

ScreenNameMasker.I.setMaskingCallback(null);
});

test('should report screen change when a route is pushed', () {
Expand All @@ -44,7 +47,7 @@ void main() {
async.elapse(const Duration(milliseconds: 1000));

verify(
mScreenLoadingManager.startUiTrace(screen),
mScreenLoadingManager.startUiTrace(screen, screen),
).called(1);

verify(
Expand All @@ -62,7 +65,7 @@ void main() {
async.elapse(const Duration(milliseconds: 1000));

verify(
mScreenLoadingManager.startUiTrace(previousScreen),
mScreenLoadingManager.startUiTrace(previousScreen, previousScreen),
).called(1);

verify(
Expand All @@ -80,14 +83,34 @@ void main() {
async.elapse(const Duration(milliseconds: 1000));

verifyNever(
mScreenLoadingManager.startUiTrace(any),
mScreenLoadingManager.startUiTrace(any, any),
);

verifyNever(
mHost.reportScreenChange(any),
);
});
});

test('should mask screen name when masking callback is set', () {
const maskedScreen = 'maskedScreen';

ScreenNameMasker.I.setMaskingCallback((_) => maskedScreen);

fakeAsync((async) {
observer.didPush(route, previousRoute);

async.elapse(const Duration(milliseconds: 1000));

verify(
mScreenLoadingManager.startUiTrace(maskedScreen, screen),
).called(1);

verify(
mHost.reportScreenChange(maskedScreen),
).called(1);
});
});
}

Route createRoute(String? name) {
Expand Down
Loading