store: Handle invalid API key on register-queue

This fixes the user reported issue by logging out the account
if the API key is found invalid:
  https://chat.zulip.org/#narrow/channel/48-mobile/topic/0.2E0.2E19.20Flutter.20.3A.20Cant.20connect.20to.20self.20hosted.20instance/near/2004042

Fixes: zulip#737

Signed-off-by: Zixuan James Li <[email protected]>

assets/l10n/app_en.arb

@@ -458,6 +458,13 @@
   "@topicValidationErrorMandatoryButEmpty": {
     "description": "Topic validation error when topic is required but was empty."
   },
+  "errorInvalidApiKeyMessage": "Your account at {url} could not be authenticated. Please try logging in again or use another account.",
+  "@errorInvalidApiKeyMessage": {
+    "description": "Error message in the dialog for invalid API key.",
+    "placeholders": {
+      "url": {"type": "String", "example": "http://chat.example.com/"}
+    }
+  },
   "errorInvalidResponse": "The server sent an invalid response",
   "@errorInvalidResponse": {
     "description": "Error message when an API call returned an invalid response."

lib/generated/l10n/zulip_localizations.dart

@@ -721,6 +721,12 @@ abstract class ZulipLocalizations {
   /// **'Topics are required in this organization.'**
   String get topicValidationErrorMandatoryButEmpty;
 
+  /// Error message in the dialog for invalid API key.
+  ///
+  /// In en, this message translates to:
+  /// **'Your account at {url} could not be authenticated. Please try logging in again or use another account.'**
+  String errorInvalidApiKeyMessage(String url);
+
   /// Error message when an API call returned an invalid response.
   ///
   /// In en, this message translates to:

lib/generated/l10n/zulip_localizations_ar.dart

@@ -357,6 +357,11 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
   @override
   String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
 
+  @override
+  String errorInvalidApiKeyMessage(String url) {
+    return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
+  }
+
   @override
   String get errorInvalidResponse => 'The server sent an invalid response';
 

lib/generated/l10n/zulip_localizations_en.dart

@@ -357,6 +357,11 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
   @override
   String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
 
+  @override
+  String errorInvalidApiKeyMessage(String url) {
+    return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
+  }
+
   @override
   String get errorInvalidResponse => 'The server sent an invalid response';
 

lib/generated/l10n/zulip_localizations_fr.dart

@@ -357,6 +357,11 @@ class ZulipLocalizationsFr extends ZulipLocalizations {
   @override
   String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
 
+  @override
+  String errorInvalidApiKeyMessage(String url) {
+    return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
+  }
+
   @override
   String get errorInvalidResponse => 'The server sent an invalid response';
 

lib/generated/l10n/zulip_localizations_ja.dart

@@ -357,6 +357,11 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
   @override
   String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
 
+  @override
+  String errorInvalidApiKeyMessage(String url) {
+    return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
+  }
+
   @override
   String get errorInvalidResponse => 'The server sent an invalid response';
 

lib/generated/l10n/zulip_localizations_pl.dart

@@ -357,6 +357,11 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
   @override
   String get topicValidationErrorMandatoryButEmpty => 'Wątki są wymagane przez tę organizację.';
 
+  @override
+  String errorInvalidApiKeyMessage(String url) {
+    return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
+  }
+
   @override
   String get errorInvalidResponse => 'Nieprawidłowa odpowiedź serwera';
 

lib/generated/l10n/zulip_localizations_ru.dart

@@ -357,6 +357,11 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
   @override
   String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
 
+  @override
+  String errorInvalidApiKeyMessage(String url) {
+    return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
+  }
+
   @override
   String get errorInvalidResponse => 'The server sent an invalid response';
 

lib/model/store.dart

@@ -19,6 +19,7 @@ import '../api/backoff.dart';
 import '../api/route/realm.dart';
 import '../log.dart';
 import '../notifications/receive.dart';
+import '../widgets/actions.dart';
 import 'autocomplete.dart';
 import 'database.dart';
 import 'emoji.dart';
@@ -149,7 +150,26 @@ abstract class GlobalStore extends ChangeNotifier {
   /// and/or [perAccountSync].
   Future<PerAccountStore> loadPerAccount(int accountId) async {
     assert(_accounts.containsKey(accountId));
-    final store = await doLoadPerAccount(accountId);
+    final PerAccountStore store;
+    try {
+      store = await doLoadPerAccount(accountId);
+    } catch (e) {
+      switch (e) {
+        case ZulipApiException(code: 'INVALID_API_KEY'):
+          // The API key is invalid and the store can never be loaded
+          // unless the user retries manually.
+          final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
+          final account = getAccount(accountId);
+          reportErrorToUserModally(
+            zulipLocalizations.errorCouldNotConnectTitle,
+            details: zulipLocalizations.errorInvalidApiKeyMessage(
+              account!.realmUrl.toString()));
+          unawaited(logOutAccount(this, accountId));
+          throw AccountNotFoundException();
+        default:
+          rethrow;
+      }
+    }
     if (!_accounts.containsKey(accountId)) {
       // [removeAccount] was called during [doLoadPerAccount].
       store.dispose();
@@ -912,7 +932,13 @@ class UpdateMachine {
         // at 1 kiB (at least on Android), and stack can be longer than that.
         assert(debugLog('Stack:\n$s'));
         assert(debugLog('Backing off, then will retry…'));
-        // TODO tell user if initial-fetch errors persist, or look non-transient
+        // TODO(#890): tell user if initial-fetch errors persist, or look non-transient
+        switch (e) {
+          case ZulipApiException(code: 'INVALID_API_KEY'):
+            // We cannot recover from this error through retrying.
+            // Leave it to [GlobalStore.loadPerAccount].
+            rethrow;
+        }
         await (backoffMachine ??= BackoffMachine()).wait();
         assert(debugLog('… Backoff wait complete, retrying initial fetch.'));
       }
@@ -1044,7 +1070,13 @@ class UpdateMachine {
       }
     } catch (e) {
       if (_disposed) return;
-      await _handlePollError(e);
+      try {
+        await _handlePollError(e);
+      } on AccountNotFoundException {
+        // Cannot recover by replacing the store because the account
+        // was logged out.
+        return;
+      }
       assert(_disposed);
       return;
     }

lib/widgets/store.dart

@@ -3,6 +3,7 @@ import 'package:flutter/scheduler.dart';
 
 import '../model/binding.dart';
 import '../model/store.dart';
+import 'app.dart';
 import 'page.dart';
 
 /// Provides access to the app's data.
@@ -228,7 +229,17 @@ class _PerAccountStoreWidgetState extends State<PerAccountStoreWidget> {
       _setStore(null);
       if (widget.routeToRemoveOnLogout != null) {
         SchedulerBinding.instance.addPostFrameCallback(
-          (_) => Navigator.of(context).removeRoute(widget.routeToRemoveOnLogout!));
+          (_) {
+            final navigator = Navigator.of(context);
+            if (widget.routeToRemoveOnLogout!.isFirst) {
+              // This ensures that we never remove the root navigator level.
+              navigator.replace(
+                oldRoute: widget.routeToRemoveOnLogout!,
+                newRoute: MaterialWidgetRoute(page: const ChooseAccountPage()));
+            } else {
+              navigator.removeRoute(widget.routeToRemoveOnLogout!);
+            }
+          });
       }
       return;
     }

test/model/store_test.dart

@@ -7,6 +7,7 @@ import 'package:flutter/foundation.dart';
 import 'package:http/http.dart' as http;
 import 'package:test/scaffolding.dart';
 import 'package:zulip/api/core.dart';
+import 'package:zulip/api/exception.dart';
 import 'package:zulip/api/model/events.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/api/route/events.dart';
@@ -500,6 +501,20 @@ void main() {
         users.map((expected) => (it) => it.fullName.equals(expected.fullName)));
     }));
 
+    test('GlobalStore.perAccount on INVALID_API_KEY', () => awaitFakeAsync((async) async {
+      addTearDown(testBinding.reset);
+
+      await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
+      testBinding.globalStore.loadPerAccountException = ZulipApiException(
+        routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
+        data: {}, message: '');
+      await check(testBinding.globalStore.perAccount(eg.selfAccount.id))
+        .throws<AccountNotFoundException>();
+
+      check(testBinding.globalStore.takeDoRemoveAccountCalls())
+        .single.equals(eg.selfAccount.id);
+    }));
+
     // TODO test UpdateMachine.load starts polling loop
     // TODO test UpdateMachine.load calls registerNotificationToken
   });
@@ -843,6 +858,27 @@ void main() {
       check(store.debugMessageListViews).isEmpty();
     }));
 
+    test('log out if fail to reload on unexpected errors', () => awaitFakeAsync((async) async {
+      await preparePoll();
+      check(globalStore.perAccountSync(store.accountId)).identicalTo(store);
+
+      prepareUnexpectedLoopError();
+      updateMachine.debugAdvanceLoop();
+      async.elapse(Duration.zero);
+      check(store).isLoading.isTrue();
+
+      globalStore.loadPerAccountException = ZulipApiException(
+        routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
+        data: {}, message: '');
+      // The reload doesn't happen immediately; there's a timer.
+      check(globalStore.perAccountSync(store.accountId)).identicalTo(store);
+      check(async.pendingTimers).length.equals(1);
+      async.flushTimers();
+
+      check(globalStore.takeDoRemoveAccountCalls().single)
+        .equals(eg.selfAccount.id);
+    }));
+
     group('report error', () {
       String? lastReportedError;
       String? takeLastReportedError() {

test/model/test_store.dart

@@ -126,6 +126,7 @@ class TestGlobalStore extends GlobalStore {
 
   static const Duration removeAccountDuration = Duration(milliseconds: 1);
   Duration? loadPerAccountDuration;
+  Object? loadPerAccountException;
 
   /// Consume the log of calls made to [doRemoveAccount].
   List<int> takeDoRemoveAccountCalls() {
@@ -147,6 +148,9 @@ class TestGlobalStore extends GlobalStore {
     if (loadPerAccountDuration != null) {
       await Future<void>.delayed(loadPerAccountDuration!);
     }
+    if (loadPerAccountException != null) {
+      throw loadPerAccountException!;
+    }
     final initialSnapshot = _initialSnapshots[accountId]!;
     final store = PerAccountStore.fromInitialSnapshot(
       globalStore: this,

test/widgets/actions_test.dart

@@ -206,7 +206,7 @@ void main() {
       check(findLoadingPage).findsNothing();
 
       final removedRoutes = <Route<dynamic>>[];
-      testNavObserver.onRemoved = (route, prevRoute) => removedRoutes.add(route);
+      testNavObserver.onReplaced = (route, prevRoute) => removedRoutes.add(prevRoute!);
 
       final context = tester.element(find.byType(MaterialApp));
       final future = logOutAccount(GlobalStoreWidget.of(context), account1.id);

test/widgets/store_test.dart

@@ -1,13 +1,23 @@
 import 'package:checks/checks.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
+import 'package:zulip/api/exception.dart';
 import 'package:zulip/model/store.dart';
+import 'package:zulip/widgets/app.dart';
+import 'package:zulip/widgets/home.dart';
+import 'package:zulip/widgets/message_list.dart';
+import 'package:zulip/widgets/page.dart';
 import 'package:zulip/widgets/store.dart';
 
 import '../flutter_checks.dart';
 import '../model/binding.dart';
 import '../example_data.dart' as eg;
 import '../model/store_checks.dart';
+import '../model/test_store.dart';
+import '../notifications/display_test.dart';
+import '../test_navigation.dart';
+import 'dialog_checks.dart';
+import 'page_checks.dart';
 
 /// A widget whose state uses [PerAccountStoreAwareStateMixin].
 class MyWidgetWithMixin extends StatefulWidget {
@@ -113,6 +123,67 @@ void main() {
       .contains('consider MaterialAccountWidgetRoute');
   });
 
+  testWidgets('PerAccountStoreWidget when log out, replace root level route', (tester) async {
+    Route<void>? newRoute, oldRoute;
+    final testNavObserver = TestNavigatorObserver()
+      ..onReplaced = (route, prevRoute) {
+      assert(newRoute == null && oldRoute == null);
+      newRoute = route!;
+      oldRoute = prevRoute!;
+    };
+    // A route to remove is a root level home route
+    testBinding.globalStore.loadPerAccountException = ZulipApiException(
+      routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
+      data: {}, message: '');
+    await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
+    await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
+    await tester.pump(); // start to load account
+    await tester.pump(); // got the error
+    await tester.pump(TestGlobalStore.removeAccountDuration);
+
+    checkErrorDialog(tester,
+      expectedTitle: 'Could not connect',
+      expectedMessage:
+        'Your account at https://chat.example/ could not be authenticated.'
+        ' Please try logging in again or use another account.');
+    check(newRoute).isA<WidgetRoute>().page.isA<ChooseAccountPage>();
+    check(oldRoute).isA<WidgetRoute>().page.isA<HomePage>();
+    check(testBinding.globalStore.takeDoRemoveAccountCalls())
+      .single.equals(eg.selfAccount.id);
+  });
+
+  testWidgets('PerAccountStoreWidget when log out, remove non-root level route', (tester) async {
+    final testNavObserver = TestNavigatorObserver();
+    await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+    await testBinding.globalStore.insertAccount(eg.otherAccount.toCompanion(false));
+    await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
+    await tester.pump(); // start to load selfAccount
+    await tester.pump(); // done loading
+
+    final pushedRoutes = <Route<void>>[];
+    final removedRoutes = <Route<void>>[];
+    testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route);
+    testNavObserver.onRemoved = (route, prevRoute) => removedRoutes.add(route);
+    testBinding.globalStore.loadPerAccountException = ZulipApiException(
+      routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
+      data: {}, message: '');
+    await openNotification(tester, eg.otherAccount, eg.streamMessage());
+    await tester.pump();
+    await tester.pump(TestGlobalStore.removeAccountDuration);
+
+    checkErrorDialog(tester,
+      expectedTitle: 'Could not connect',
+      expectedMessage:
+        'Your account at https://chat.example/ could not be authenticated.'
+        ' Please try logging in again or use another account.');
+    check(testBinding.globalStore.takeDoRemoveAccountCalls())
+      .single.equals(eg.otherAccount.id);
+    check(pushedRoutes).deepEquals(<Condition<Object?>>[
+      (it) => it.isA<WidgetRoute>().page.isA<MessageListPage>(),
+      (it) => it.isA<DialogRoute<void>>(),
+    ]);
+  });
+
   testWidgets('PerAccountStoreWidget immediate data after first loaded', (tester) async {
     await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
     addTearDown(testBinding.reset);