From 012485118d5fdb1bf346074be0fab3a766a211cf Mon Sep 17 00:00:00 2001 From: Michael Markl Date: Thu, 26 Sep 2024 18:38:19 +0200 Subject: [PATCH] Report error to sentry if initialization of userCodeModel fails. Also, if initialization fails, do not implicitly use empty array of user codes, but show error. Also throw Errors when using UserCodeModel when it is not properly initialized. Also add a loading spinner and some error popup to deeplink activation. --- frontend/assets/l10n/app_de.json | 3 +- frontend/assets/l10n/app_en.json | 3 +- frontend/lib/about/backend_switch_dialog.dart | 12 +- frontend/lib/about/dev_settings_view.dart | 2 +- .../lib/activation/deeplink_activation.dart | 136 ++++++++++++------ .../activation_code_scanner_page.dart | 5 +- .../activation_existing_card_dialog.dart | 17 +-- .../identification/identification_page.dart | 4 + .../lib/identification/user_code_model.dart | 65 ++++++--- .../lib/identification/user_code_store.dart | 13 +- .../identification/util/activate_card.dart | 22 +-- 11 files changed, 190 insertions(+), 92 deletions(-) diff --git a/frontend/assets/l10n/app_de.json b/frontend/assets/l10n/app_de.json index 2352f091f..8c3e85b10 100644 --- a/frontend/assets/l10n/app_de.json +++ b/frontend/assets/l10n/app_de.json @@ -66,7 +66,8 @@ "openSettings": "Einstellungen öffnen", "previous": "Zurück", "settings": "Einstellungen", - "tryAgain": "Erneut versuchen" + "tryAgain": "Erneut versuchen", + "unknownError": "Ein unbekannter Fehler ist aufgetreten." }, "deeplinkActivation": { "headline": "Karte aktivieren", diff --git a/frontend/assets/l10n/app_en.json b/frontend/assets/l10n/app_en.json index f453e6a22..3b08b253e 100644 --- a/frontend/assets/l10n/app_en.json +++ b/frontend/assets/l10n/app_en.json @@ -76,7 +76,8 @@ "openSettings": "Open settings", "previous": "Back", "settings": "Settings", - "tryAgain": "Try again" + "tryAgain": "Try again", + "unknownError": "An unknown error occurred." }, "identification": { "activate": "Activate", diff --git a/frontend/lib/about/backend_switch_dialog.dart b/frontend/lib/about/backend_switch_dialog.dart index ba04b5143..f8e82d329 100644 --- a/frontend/lib/about/backend_switch_dialog.dart +++ b/frontend/lib/about/backend_switch_dialog.dart @@ -74,18 +74,18 @@ class BackendSwitchDialogState extends State { ); } - void switchBackendUrl(BuildContext context) { + Future switchBackendUrl(BuildContext context) async { final settings = Provider.of(context, listen: false); final updatedEnableStaging = !settings.enableStaging; - clearData(); - settings.setEnableStaging(enabled: updatedEnableStaging); + await clearData(); + await settings.setEnableStaging(enabled: updatedEnableStaging); Navigator.of(context, rootNavigator: true).pop(); } - void clearData() { + Future clearData() async { final settings = Provider.of(context, listen: false); final userCodesModel = Provider.of(context, listen: false); - settings.clearSettings(); - userCodesModel.removeCodes(); + await settings.clearSettings(); + await userCodesModel.removeCodes(); } } diff --git a/frontend/lib/about/dev_settings_view.dart b/frontend/lib/about/dev_settings_view.dart index 35b851280..d2a8f03b0 100644 --- a/frontend/lib/about/dev_settings_view.dart +++ b/frontend/lib/about/dev_settings_view.dart @@ -100,7 +100,7 @@ class DevSettingsView extends StatelessWidget { } Future _resetEakData(BuildContext context, UserCodeModel userCodesModel) async { - userCodesModel.removeCodes(); + await userCodesModel.removeCodes(); } DynamicUserCode _determineUserCode(String projectId) { diff --git a/frontend/lib/activation/deeplink_activation.dart b/frontend/lib/activation/deeplink_activation.dart index e3c9c04ca..cb9acf072 100644 --- a/frontend/lib/activation/deeplink_activation.dart +++ b/frontend/lib/activation/deeplink_activation.dart @@ -41,17 +41,38 @@ enum DeepLinkActivationStatus { } } -class DeepLinkActivation extends StatelessWidget { +class DeepLinkActivation extends StatefulWidget { final String base64qrcode; const DeepLinkActivation({super.key, required this.base64qrcode}); + @override + State createState() => _DeepLinkActivationState(); +} + +enum _State { + waiting, + loading, + success, +} + +class _DeepLinkActivationState extends State { + _State _state = _State.waiting; + @override Widget build(BuildContext context) { final t = context.t; - DynamicActivationCode? activationCode = getActivationCode(context, base64qrcode); + DynamicActivationCode? activationCode = getActivationCode(context, widget.base64qrcode); CardInfo? cardInfo = activationCode?.info; - final userCodeModel = Provider.of(context, listen: false); + final userCodeModel = Provider.of(context); + + if (!userCodeModel.isInitialized) { + if (userCodeModel.initializationFailed) { + return SafeArea(child: Center(child: Text(t.common.unknownError, textAlign: TextAlign.center))); + } + return Container(); + } + final status = DeepLinkActivationStatus.from(userCodeModel, activationCode); final regionId = cardInfo?.extensions.extensionRegion.regionId ?? -1; final regionsQuery = GetRegionsByIdQuery( @@ -60,9 +81,6 @@ class DeepLinkActivation extends StatelessWidget { ids: [regionId], ), ); - - final String cardsInUse = userCodeModel.userCodes.length.toString(); - final String maxCardAmount = buildConfig.maxCardAmount.toString(); final theme = Theme.of(context); return Query( @@ -97,34 +115,56 @@ class DeepLinkActivation extends StatelessWidget { )), Padding( padding: const EdgeInsets.all(32), - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - ...switch (status) { - DeepLinkActivationStatus.invalidLink => [_WarningText(t.deeplinkActivation.invalidCode)], - DeepLinkActivationStatus.limitReached => [ - _WarningText('${t.deeplinkActivation.limitReached} ($cardsInUse/$maxCardAmount)') - ], - DeepLinkActivationStatus.alreadyExists => [ - _WarningText(t.deeplinkActivation.alreadyExists) - ], - DeepLinkActivationStatus.valid => [ - ElevatedButton( - onPressed: activationCode != null - ? () { - activateCard( - context, - () => GoRouter.of(context) - .pushReplacement('$homeRouteName/$identityTabIndex'), - activationCode); - } - : null, - child: Text(t.deeplinkActivation.buttonText), + child: Column(mainAxisSize: MainAxisSize.min, children: [ + if (_state == _State.waiting) _WarningText(status, userCodeModel), + ElevatedButton.icon( + onPressed: activationCode != null && + _state == _State.waiting && + status == DeepLinkActivationStatus.valid + ? () async { + setState(() { + _state = _State.loading; + }); + try { + final activated = await activateCard(context, activationCode); + if (activated) { + GoRouter.of(context).pushReplacement('$homeRouteName/$identityTabIndex'); + setState(() { + _state = _State.success; + }); + } else { + setState(() { + _state = _State.waiting; + }); + } + } catch (_) { + setState(() { + _state = _State.waiting; + }); + // TODO 1656: Improve error handling!! + ScaffoldMessenger.of(context).showSnackBar( + SnackBar( + backgroundColor: Theme.of(context).colorScheme.primary, + content: Text(t.common.unknownError), + ), + ); + rethrow; + } + } + : null, + icon: _state != _State.waiting + ? Container( + width: 24, + height: 24, + padding: const EdgeInsets.all(2.0), + child: const CircularProgressIndicator( + strokeWidth: 3, + ), ) - ], - }, - ], - ), + : const Icon(Icons.add_card), + label: Text(t.deeplinkActivation.buttonText), + ) + ]), ), ], )), @@ -134,18 +174,32 @@ class DeepLinkActivation extends StatelessWidget { } class _WarningText extends StatelessWidget { - final String text; + final DeepLinkActivationStatus status; + final UserCodeModel userCodeModel; - const _WarningText(this.text); + const _WarningText(this.status, this.userCodeModel); @override Widget build(BuildContext context) { - return Column( - children: [ - Icon(Icons.warning, color: Theme.of(context).colorScheme.secondary), - Text(text, textAlign: TextAlign.center) - ], - ); + final String cardsInUse = userCodeModel.userCodes.length.toString(); + final String maxCardAmount = buildConfig.maxCardAmount.toString(); + final text = switch (status) { + DeepLinkActivationStatus.invalidLink => t.deeplinkActivation.activationInvalid, + DeepLinkActivationStatus.limitReached => '${t.deeplinkActivation.limitReached} ($cardsInUse/$maxCardAmount)', + DeepLinkActivationStatus.alreadyExists => t.deeplinkActivation.alreadyExists, + DeepLinkActivationStatus.valid => '', + }; + if (status == DeepLinkActivationStatus.valid) { + return Container(); + } + return Padding( + padding: EdgeInsets.symmetric(vertical: 8), + child: Column( + children: [ + Icon(Icons.warning, color: Theme.of(context).colorScheme.secondary), + Text(text, textAlign: TextAlign.center) + ], + )); } } diff --git a/frontend/lib/identification/activation_workflow/activation_code_scanner_page.dart b/frontend/lib/identification/activation_workflow/activation_code_scanner_page.dart index 99b1bc96b..714562288 100644 --- a/frontend/lib/identification/activation_workflow/activation_code_scanner_page.dart +++ b/frontend/lib/identification/activation_workflow/activation_code_scanner_page.dart @@ -42,7 +42,10 @@ class ActivationCodeScannerPage extends StatelessWidget { try { final activationCode = const ActivationCodeParser().parseQrCodeContent(code); - await activateCard(context, moveToLastCard, activationCode); + final activated = await activateCard(context, activationCode); + if (activated) { + moveToLastCard(); + } } on ActivationDidNotOverwriteExisting catch (_) { await showError(t.identification.cardAlreadyActivated, null); } on QrCodeFieldMissingException catch (e) { diff --git a/frontend/lib/identification/activation_workflow/activation_existing_card_dialog.dart b/frontend/lib/identification/activation_workflow/activation_existing_card_dialog.dart index 630dd5521..bfb1eec2e 100644 --- a/frontend/lib/identification/activation_workflow/activation_existing_card_dialog.dart +++ b/frontend/lib/identification/activation_workflow/activation_existing_card_dialog.dart @@ -1,5 +1,7 @@ import 'package:flutter/material.dart'; +import 'package:ehrenamtskarte/l10n/translations.g.dart'; + class ActivationExistingCardDialog extends StatelessWidget { const ActivationExistingCardDialog({super.key}); @@ -18,9 +20,9 @@ class ActivationExistingCardDialog extends StatelessWidget { ), actions: [ TextButton( - child: const Text('Ok'), + child: Text(context.t.common.ok), onPressed: () { - Navigator.of(context).pop(false); + Navigator.of(context).pop(); }, ) ], @@ -28,11 +30,10 @@ class ActivationExistingCardDialog extends StatelessWidget { } /// Returns true, if the user wants to activate an existing card - static Future showExistingCardDialog(BuildContext context) async { - return await showDialog( - context: context, - builder: (context) => ActivationExistingCardDialog(), - ) ?? - false; + static Future showExistingCardDialog(BuildContext context) async { + return await showDialog( + context: context, + builder: (context) => ActivationExistingCardDialog(), + ); } } diff --git a/frontend/lib/identification/identification_page.dart b/frontend/lib/identification/identification_page.dart index 5a23d8720..395a728bf 100644 --- a/frontend/lib/identification/identification_page.dart +++ b/frontend/lib/identification/identification_page.dart @@ -9,6 +9,7 @@ import 'package:ehrenamtskarte/identification/qr_code_scanner/qr_code_camera_per import 'package:ehrenamtskarte/identification/user_code_model.dart'; import 'package:ehrenamtskarte/identification/verification_workflow/dialogs/remove_card_confirmation_dialog.dart'; import 'package:ehrenamtskarte/identification/verification_workflow/verification_workflow.dart'; +import 'package:ehrenamtskarte/l10n/translations.g.dart'; import 'package:ehrenamtskarte/proto/card.pb.dart'; import 'package:ehrenamtskarte/routing.dart'; import 'package:flutter/material.dart'; @@ -34,6 +35,9 @@ class IdentificationPageState extends State { return Consumer( builder: (context, userCodeModel, child) { if (!userCodeModel.isInitialized) { + if (userCodeModel.initializationFailed) { + return SafeArea(child: Center(child: Text(context.t.common.unknownError, textAlign: TextAlign.center))); + } return Container(); } diff --git a/frontend/lib/identification/user_code_model.dart b/frontend/lib/identification/user_code_model.dart index 8b7a7bbdc..ae84a8d22 100644 --- a/frontend/lib/identification/user_code_model.dart +++ b/frontend/lib/identification/user_code_model.dart @@ -1,38 +1,60 @@ -import 'dart:developer'; +import 'dart:collection'; import 'package:ehrenamtskarte/build_config/build_config.dart'; import 'package:ehrenamtskarte/identification/user_code_store.dart'; import 'package:ehrenamtskarte/proto/card.pb.dart'; import 'package:flutter/foundation.dart'; +import 'package:ehrenamtskarte/sentry.dart'; + +enum _InitializationState { uninitialized, initializing, failed, initialized } + class UserCodeModel extends ChangeNotifier { List _userCodes = []; - bool _isInitialized = false; + _InitializationState _initState = _InitializationState.uninitialized; List get userCodes { - return _userCodes; + if (!isInitialized) { + throw StateError('UserCodeModel not initialized!'); + } + return UnmodifiableListView(_userCodes); } bool get isInitialized { - return _isInitialized; + return _initState == _InitializationState.initialized; + } + + bool get initializationFailed { + return _initState == _InitializationState.failed; } Future initialize() async { - if (_isInitialized) { + if (_initState == _InitializationState.initializing || _initState == _InitializationState.initialized) { return; } + _initState = _InitializationState.initializing; + const store = UserCodeStore(); try { - await UserCodeStore().importLegacyCard(); - _userCodes = await const UserCodeStore().load(); - } on Exception catch (e) { - log('Failed to initialize activation code from secure storage.', error: e); + DynamicUserCode? legacyCode = await store.loadAndDeleteLegacyCard(); + var userCodes = await store.load(); + if (legacyCode != null && !isAlreadyInList(userCodes, legacyCode.info)) { + userCodes.add(legacyCode); + await store.store(userCodes); + } + _userCodes = userCodes; + _initState = _InitializationState.initialized; + } catch (e, stackTrace) { + reportError('Failed to initialize activation code from secure storage: $e', stackTrace); + _initState = _InitializationState.failed; } finally { - _isInitialized = true; notifyListeners(); } } void insertCode(DynamicUserCode code) { + if (!isInitialized) { + throw StateError('UserCodeModel not initialized!'); + } List userCodes = _userCodes; if (isAlreadyInList(userCodes, code.info)) return; userCodes.add(code); @@ -42,9 +64,12 @@ class UserCodeModel extends ChangeNotifier { } void updateCode(DynamicUserCode code) { + if (!isInitialized) { + throw StateError('UserCodeModel not initialized!'); + } List userCodes = _userCodes; if (isAlreadyInList(userCodes, code.info)) { - userCodes = updateUserCode(userCodes, code); + userCodes = _updateUserCode(userCodes, code); const UserCodeStore().store(userCodes); _userCodes = userCodes; notifyListeners(); @@ -52,6 +77,9 @@ class UserCodeModel extends ChangeNotifier { } void removeCode(DynamicUserCode code) { + if (!isInitialized) { + throw StateError('UserCodeModel not initialized!'); + } List userCodes = _userCodes; userCodes.remove(code); const UserCodeStore().store(userCodes); @@ -59,16 +87,19 @@ class UserCodeModel extends ChangeNotifier { notifyListeners(); } - void removeCodes() { - const UserCodeStore().remove(); + Future removeCodes() async { + if (!isInitialized) { + throw StateError('UserCodeModel not initialized!'); + } + await const UserCodeStore().remove(); _userCodes = []; notifyListeners(); } +} - List updateUserCode(List userCodes, DynamicUserCode userCode) { - userCodes[userCodes.indexWhere((code) => code.info == userCode.info)] = userCode; - return userCodes; - } +List _updateUserCode(List userCodes, DynamicUserCode userCode) { + userCodes[userCodes.indexWhere((code) => code.info == userCode.info)] = userCode; + return userCodes; } bool isAlreadyInList(List userCodes, CardInfo info) { diff --git a/frontend/lib/identification/user_code_store.dart b/frontend/lib/identification/user_code_store.dart index ce420f935..ddf67940b 100644 --- a/frontend/lib/identification/user_code_store.dart +++ b/frontend/lib/identification/user_code_store.dart @@ -1,7 +1,6 @@ import 'dart:convert'; import 'dart:io'; -import 'package:ehrenamtskarte/identification/user_code_model.dart'; import 'package:ehrenamtskarte/proto/card.pb.dart'; import 'package:ehrenamtskarte/sentry.dart'; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; @@ -12,7 +11,7 @@ class UserCodeStore { static const _userCodesBase64Key = 'userCodesBase64'; // legacy key for single card - static const _userCodeBase64Key = 'userCodeBase64'; + static const _legacyUserCodeBase64Key = 'userCodeBase64'; static const _storageDelimiter = ','; Future store(List userCodes) async { @@ -53,12 +52,12 @@ class UserCodeStore { } // legacy import of existing card to keep them in storage after update - Future importLegacyCard() async { + Future loadAndDeleteLegacyCard() async { const storage = FlutterSecureStorage(); - final String? userCodeBase64 = await storage.read(key: _userCodeBase64Key); - if (userCodeBase64 == null) return; + final String? userCodeBase64 = await storage.read(key: _legacyUserCodeBase64Key); + if (userCodeBase64 == null) return null; DynamicUserCode importedLegacyCard = DynamicUserCode.fromBuffer(const Base64Decoder().convert(userCodeBase64)); - UserCodeModel().insertCode(importedLegacyCard); - await storage.delete(key: _userCodeBase64Key); + await storage.delete(key: _legacyUserCodeBase64Key); + return importedLegacyCard; } } diff --git a/frontend/lib/identification/util/activate_card.dart b/frontend/lib/identification/util/activate_card.dart index 61ecbc088..15f737b67 100644 --- a/frontend/lib/identification/util/activate_card.dart +++ b/frontend/lib/identification/util/activate_card.dart @@ -18,9 +18,12 @@ import 'package:provider/provider.dart'; import 'package:ehrenamtskarte/l10n/translations.g.dart'; -Future activateCard( +/// Returns +/// - `true`, if the activation was successful, +/// - `false`, if the activation was not successful, but feedback was given to the user, +/// Throws an error otherwise. +Future activateCard( BuildContext context, - VoidCallback onSuccessfulCardActivation, DynamicActivationCode activationCode, [ bool overwriteExisting = false, ]) async { @@ -30,6 +33,7 @@ Future activateCard( final activationSecretBase64 = const Base64Encoder().convert(activationCode.activationSecret); final cardInfoBase64 = activationCode.info.hash(activationCode.pepper); final messengerState = ScaffoldMessenger.of(context); + final t = context.t; debugPrint('Card Activation: Sending request with overwriteExisting=$overwriteExisting.'); @@ -58,36 +62,36 @@ Future activateCard( ..verificationTimeStamp = secondsSinceEpoch(DateTime.parse(activationResult.activationTimeStamp))); userCodesModel.insertCode(userCode); + debugPrint('Card Activation: Successfully activated.'); messengerState.showSnackBar( SnackBar( backgroundColor: Theme.of(context).colorScheme.primary, content: Text(t.deeplinkActivation.activationSuccessful), ), ); - onSuccessfulCardActivation(); - debugPrint('Card Activation: Successfully activated.'); if (Navigator.canPop(context)) Navigator.maybePop(context); - break; + return true; case ActivationState.failed: await QrParsingErrorDialog.showErrorDialog( context, t.identification.codeInvalid, ); - break; + return false; case ActivationState.didNotOverwriteExisting: if (overwriteExisting) { throw const ActivationDidNotOverwriteExisting(); } if (isAlreadyInList(userCodesModel.userCodes, activationCode.info)) { await ActivationExistingCardDialog.showExistingCardDialog(context); - break; + return false; } debugPrint( 'Card Activation: Card had been activated already and was not overwritten. Waiting for user feedback.'); if (await ActivationOverwriteExistingDialog.showActivationOverwriteExistingDialog(context)) { - await activateCard(context, onSuccessfulCardActivation, activationCode, overwriteExisting = true); + return await activateCard(context, activationCode, overwriteExisting = true); + } else { + return false; } - break; default: const errorMessage = 'Die Aktivierung befindet sich in einem ungültigen Zustand.'; reportError(errorMessage, null);