feat(recoverbull): warn before saving a custom key server

Removing the permission page took the custom-server warning with it. Put
it back where it is actually useful: at the moment the user chooses a
server, rather than on a screen they walk past afterwards.

Saving a URL that is neither the default nor the server already in use now
opens the shared warning sheet, and the URL is stored only if the user
confirms. Dismissing stores nothing and drops back to showing the server
in use, so nobody can end up pointed at someone else's key server without
having read the warning. Returning to the default is the safe direction
and passes straight through.

Validation still runs first: a malformed URL is rejected before any
warning, so the user is never asked to confirm something the app is about
to refuse.

Author: FrancisPouliot

lib/features/recoverbull/ui/pages/settings_page.dart

@@ -2,9 +2,11 @@ import 'package:bb_mobile/core/recoverbull/domain/usecases/fetch_recoverbull_url
 import 'package:bb_mobile/core/recoverbull/domain/usecases/store_recoverbull_url_usecase.dart';
 import 'package:bb_mobile/core/themes/app_theme.dart';
 import 'package:bb_mobile/core/utils/build_context_x.dart';
+import 'package:bb_mobile/core/utils/constants.dart';
 import 'package:bb_mobile/core/utils/logger.dart';
 import 'package:bb_mobile/core/widgets/buttons/button.dart';
 import 'package:bb_mobile/core/widgets/text/text.dart';
+import 'package:bb_mobile/core/widgets/warning_bottom_sheet.dart';
 import 'package:bb_mobile/locator.dart';
 import 'package:flutter/material.dart';
 import 'package:gap/gap.dart';
@@ -53,8 +55,23 @@ class _SettingsPageState extends State<SettingsPage> {
   }
 
   Future<void> _saveUrl() async {
+    // Validate first: nobody should confirm a security warning only to be told
+    // the URL was malformed.
     if (!_formKey.currentState!.validate()) return;
 
+    // Warn at the moment of choice, and only in the unsafe direction. Adopting
+    // a key server we do not run is the risk; returning to the default, or
+    // re-saving the server already in use, needs no ceremony.
+    final entered = _urlController.text;
+    final isNewCustomServer =
+        entered != SettingsConstants.recoverbullUrl && entered != _originalUrl;
+    if (isNewCustomServer && !await _confirmCustomServer()) {
+      // Nothing is stored, and the field goes back to the server in use.
+      if (mounted) _cancelEdit();
+      return;
+    }
+    if (!mounted) return;
+
     setState(() => _isSaving = true);
     try {
       final url = Uri.parse(_urlController.text);
@@ -70,6 +87,18 @@ class _SettingsPageState extends State<SettingsPage> {
     }
   }
 
+  Future<bool> _confirmCustomServer() async {
+    var confirmed = false;
+    await WarningBottomSheet.show(
+      context,
+      title: context.loc.securityWarningTitle,
+      message: context.loc.recoverbullServerCustomWarning,
+      confirmLabel: context.loc.recoverbullContinue,
+      onConfirm: () => confirmed = true,
+    );
+    return confirmed;
+  }
+
   void _cancelEdit() => setState(() => _isEditing = false);
 
   Future<void> _openRecoverBullWebsite() async {

localization/app_en.arb

@@ -15065,5 +15065,9 @@
   "backupSettingsStartBackup": "Start Backup",
   "@backupSettingsStartBackup": {
     "description": "Button text to start new backup process"
+  },
+  "recoverbullServerCustomWarning": "You are about to use a custom Recoverbull server. Make sure you trust this server.",
+  "@recoverbullServerCustomWarning": {
+    "description": "Warning shown before saving a custom key server URL in the encrypted vault settings"
   }
 }

test/features/recoverbull/recoverbull_settings_page_test.dart

@@ -0,0 +1,136 @@
+import 'package:bb_mobile/core/recoverbull/domain/usecases/fetch_recoverbull_url_usecase.dart';
+import 'package:bb_mobile/core/recoverbull/domain/usecases/store_recoverbull_url_usecase.dart';
+import 'package:bb_mobile/core/utils/constants.dart';
+import 'package:bb_mobile/features/recoverbull/ui/pages/settings_page.dart';
+import 'package:bb_mobile/generated/l10n/localization.dart';
+import 'package:bb_mobile/locator.dart';
+import 'package:flutter/material.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:go_router/go_router.dart';
+import 'package:mocktail/mocktail.dart';
+
+class _MockFetchUrlUsecase extends Mock implements FetchRecoverbullUrlUsecase {}
+
+class _MockStoreUrlUsecase extends Mock implements StoreRecoverbullUrlUsecase {}
+
+void main() {
+  const defaultUrl = SettingsConstants.recoverbullUrl;
+  const customUrl =
+      'http://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.onion';
+  const otherCustomUrl =
+      'http://bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.onion';
+
+  late AppLocalizations loc;
+  late _MockFetchUrlUsecase fetchUrl;
+  late _MockStoreUrlUsecase storeUrl;
+
+  setUpAll(() async {
+    loc = await AppLocalizations.delegate.load(const Locale('en'));
+    registerFallbackValue(Uri.parse(defaultUrl));
+  });
+
+  setUp(() {
+    fetchUrl = _MockFetchUrlUsecase();
+    storeUrl = _MockStoreUrlUsecase();
+    when(() => storeUrl.execute(any())).thenAnswer((_) async {});
+    locator
+      ..registerFactory<FetchRecoverbullUrlUsecase>(() => fetchUrl)
+      ..registerFactory<StoreRecoverbullUrlUsecase>(() => storeUrl);
+  });
+
+  tearDown(() async => locator.reset());
+
+  /// Opens the page with [current] as the stored server and types [entered]
+  /// into the URL field, then presses Save.
+  Future<void> editAndSave(
+    WidgetTester tester, {
+    required String current,
+    required String entered,
+  }) async {
+    when(() => fetchUrl.execute()).thenAnswer((_) async => Uri.parse(current));
+
+    await tester.pumpWidget(
+      MaterialApp.router(
+        localizationsDelegates: AppLocalizations.localizationsDelegates,
+        supportedLocales: AppLocalizations.supportedLocales,
+        routerConfig: GoRouter(
+          routes: [GoRoute(path: '/', builder: (_, _) => const SettingsPage())],
+        ),
+      ),
+    );
+    await tester.pumpAndSettle();
+
+    await tester.tap(find.text(loc.recoverbullSettingsEdit));
+    await tester.pumpAndSettle();
+    await tester.enterText(find.byType(TextFormField), entered);
+    await tester.tap(find.text(loc.recoverbullSettingsSave));
+    await tester.pumpAndSettle();
+  }
+
+  testWidgets('a new custom server is only stored once the warning is '
+      'confirmed', (tester) async {
+    await editAndSave(tester, current: defaultUrl, entered: customUrl);
+
+    expect(find.text(loc.securityWarningTitle), findsOneWidget);
+    expect(find.text(loc.recoverbullServerCustomWarning), findsOneWidget);
+    verifyNever(() => storeUrl.execute(any()));
+
+    await tester.tap(find.text(loc.recoverbullContinue));
+    await tester.pumpAndSettle();
+
+    verify(() => storeUrl.execute(Uri.parse(customUrl))).called(1);
+  });
+
+  testWidgets('dismissing the warning stores nothing and keeps the server in '
+      'use', (tester) async {
+    await editAndSave(tester, current: defaultUrl, entered: customUrl);
+    expect(find.text(loc.securityWarningTitle), findsOneWidget);
+
+    // Tap the barrier: the same escape a user gets from any bottom sheet.
+    await tester.tapAt(const Offset(10, 10));
+    await tester.pumpAndSettle();
+
+    verifyNever(() => storeUrl.execute(any()));
+    expect(find.text(loc.securityWarningTitle), findsNothing);
+    // Back out of editing, showing the server that is actually in use.
+    expect(find.text(defaultUrl), findsOneWidget);
+    expect(find.text(customUrl), findsNothing);
+  });
+
+  testWidgets('returning to the default server needs no warning', (
+    tester,
+  ) async {
+    await editAndSave(tester, current: customUrl, entered: defaultUrl);
+
+    expect(find.text(loc.securityWarningTitle), findsNothing);
+    verify(() => storeUrl.execute(Uri.parse(defaultUrl))).called(1);
+  });
+
+  testWidgets('re-saving the server already in use does not re-prompt', (
+    tester,
+  ) async {
+    await editAndSave(tester, current: customUrl, entered: customUrl);
+
+    expect(find.text(loc.securityWarningTitle), findsNothing);
+    verify(() => storeUrl.execute(Uri.parse(customUrl))).called(1);
+  });
+
+  testWidgets('swapping one custom server for another still warns', (
+    tester,
+  ) async {
+    await editAndSave(tester, current: customUrl, entered: otherCustomUrl);
+
+    expect(find.text(loc.securityWarningTitle), findsOneWidget);
+    verifyNever(() => storeUrl.execute(any()));
+  });
+
+  testWidgets('an invalid URL is rejected before the warning appears', (
+    tester,
+  ) async {
+    await editAndSave(tester, current: defaultUrl, entered: 'https://evil.com');
+
+    expect(find.text(loc.recoverbullSettingsUrlMustBeHttp), findsOneWidget);
+    expect(find.text(loc.securityWarningTitle), findsNothing);
+    verifyNever(() => storeUrl.execute(any()));
+  });
+}