From b1425a52852aa4bd3b5057f879da87bd6b34a88b Mon Sep 17 00:00:00 2001 From: Adam Velebil Date: Fri, 10 Jun 2022 12:23:52 +0200 Subject: [PATCH] YADESK-602 android and nfc exception handling --- .../authenticator/UserCancelledException.kt | 3 + .../com/yubico/authenticator/logging/Log.kt | 2 +- .../yubico/authenticator/oath/OathManager.kt | 18 ++-- lib/android/logger.dart | 9 +- lib/android/oath/state.dart | 92 +++++++++++-------- lib/app/views/user_interaction.dart | 6 +- lib/oath/views/account_mixin.dart | 9 +- lib/oath/views/account_view.dart | 23 +++-- lib/oath/views/add_account_page.dart | 5 +- lib/oath/views/delete_account_dialog.dart | 25 +++-- lib/oath/views/rename_account_dialog.dart | 24 +++-- lib/user_cancelled_exception.dart | 9 ++ 12 files changed, 150 insertions(+), 75 deletions(-) create mode 100644 android/app/src/main/kotlin/com/yubico/authenticator/UserCancelledException.kt create mode 100644 lib/user_cancelled_exception.dart diff --git a/android/app/src/main/kotlin/com/yubico/authenticator/UserCancelledException.kt b/android/app/src/main/kotlin/com/yubico/authenticator/UserCancelledException.kt new file mode 100644 index 00000000..92261d0b --- /dev/null +++ b/android/app/src/main/kotlin/com/yubico/authenticator/UserCancelledException.kt @@ -0,0 +1,3 @@ +package com.yubico.authenticator + +class UserCancelledException : Exception() \ No newline at end of file diff --git a/android/app/src/main/kotlin/com/yubico/authenticator/logging/Log.kt b/android/app/src/main/kotlin/com/yubico/authenticator/logging/Log.kt index 77dcbccf..0141f18a 100644 --- a/android/app/src/main/kotlin/com/yubico/authenticator/logging/Log.kt +++ b/android/app/src/main/kotlin/com/yubico/authenticator/logging/Log.kt @@ -76,7 +76,7 @@ object Log { } error?.let { - Log.e(TAG, "[$loggerName] ${level.name}: $error".also { + Log.e(TAG, "[$loggerName] ${level.name}(details): $error".also { _buffer.add(it) }) } diff --git a/android/app/src/main/kotlin/com/yubico/authenticator/oath/OathManager.kt b/android/app/src/main/kotlin/com/yubico/authenticator/oath/OathManager.kt index 9297835f..2846b7fd 100644 --- a/android/app/src/main/kotlin/com/yubico/authenticator/oath/OathManager.kt +++ b/android/app/src/main/kotlin/com/yubico/authenticator/oath/OathManager.kt @@ -300,12 +300,16 @@ class OathManager( override fun deleteAccount(uri: String, result: Result) { coroutineScope.launch { - useOathSession("Delete account", true) { session -> - withUnlockedSession(session) { - val credential = getOathCredential(session, uri) - session.deleteCredential(credential) - returnSuccess(result) + try { + useOathSession("Delete account", true) { session -> + withUnlockedSession(session) { + val credential = getOathCredential(session, uri) + session.deleteCredential(credential) + returnSuccess(result) + } } + } catch (cause: Throwable) { + returnError(result, cause) } } } @@ -516,7 +520,9 @@ class OathManager( dialogManager.showDialog(title) { coroutineScope.launch(Dispatchers.Main) { Log.d(TAG, "Cancelled Dialog $title") - provideYubiKey(com.yubico.yubikit.core.util.Result.failure(Exception("User canceled"))) + provideYubiKey(com.yubico.yubikit.core.util.Result.failure( + UserCancelledException() + )) } } } diff --git a/lib/android/logger.dart b/lib/android/logger.dart index 9028af08..dc46297e 100644 --- a/lib/android/logger.dart +++ b/lib/android/logger.dart @@ -36,11 +36,18 @@ class AndroidLogger extends LogLevelNotifier { } void log(LogRecord record) { + final error = record.error == null + ? null + : record.error is Exception + ? record.error.toString() + : record.error is String + ? record.error + : 'Invalid error type: ${record.error.runtimeType.toString()}'; _channel.invokeMethod('log', { 'loggerName': record.loggerName, 'level': record.level.name, 'message': record.message, - 'error': record.error + 'error': error }); } } diff --git a/lib/android/oath/state.dart b/lib/android/oath/state.dart index 7e3d7847..fd480eae 100755 --- a/lib/android/oath/state.dart +++ b/lib/android/oath/state.dart @@ -14,12 +14,11 @@ import 'package:yubico_authenticator/core/models.dart'; import 'package:yubico_authenticator/oath/state.dart'; import '../../oath/models.dart'; +import '../../user_cancelled_exception.dart'; import 'command_providers.dart'; final _log = Logger('android.oath.state'); -class CancelException implements Exception {} - final oathApiProvider = StateProvider((_) => OathApi()); final androidOathStateProvider = StateNotifierProvider.autoDispose @@ -159,44 +158,59 @@ class _AndroidCredentialListNotifier extends OathCredentialListNotifier { @override Future calculate(OathCredential credential, {bool update = true}) async { - final OathCode code; - var resultJson = await _api.calculate(credential.id); - var result = jsonDecode(resultJson); - code = OathCode.fromJson(result); - _log.debug('Calculate', jsonEncode(code)); - if (update && mounted) { - final creds = state!.toList(); - final i = creds.indexWhere((e) => e.credential.id == credential.id); - state = creds..[i] = creds[i].copyWith(code: code); + try { + var resultJson = await _api.calculate(credential.id); + var result = jsonDecode(resultJson); + final OathCode code = OathCode.fromJson(result); + _log.debug('Calculate', jsonEncode(code)); + if (update && mounted) { + final creds = state!.toList(); + final i = creds.indexWhere((e) => e.credential.id == credential.id); + state = creds..[i] = creds[i].copyWith(code: code); + } + return code; + } on PlatformException catch (pe) { + if (UserCancelledException.isCancellation(pe)) { + throw UserCancelledException(); + } + rethrow; } - return code; } @override Future addAccount(Uri credentialUri, {bool requireTouch = false}) async { - String resultString = - await _api.addAccount(credentialUri.toString(), requireTouch); + try { + String resultString = + await _api.addAccount(credentialUri.toString(), requireTouch); - var result = jsonDecode(resultString); - var addedCredential = OathCredential.fromJson(result['credential']); - var addedCredCode = OathCode.fromJson(result['code']); + var result = jsonDecode(resultString); + var addedCredential = OathCredential.fromJson(result['credential']); + var addedCredCode = OathCode.fromJson(result['code']); - if (mounted) { - final newState = state!.toList(); - final index = newState.indexWhere((e) => e.credential == addedCredential); - if (index > -1) { - newState.removeAt(index); + if (mounted) { + final newState = state!.toList(); + final index = + newState.indexWhere((e) => e.credential == addedCredential); + if (index > -1) { + newState.removeAt(index); + } + newState.add(OathPair( + addedCredential, + addedCredCode, + )); + state = newState; } - newState.add(OathPair( - addedCredential, - addedCredCode, - )); - state = newState; - } - refresh(); - return addedCredential; + refresh(); + return addedCredential; + } on PlatformException catch (pe) { + if (UserCancelledException.isCancellation(pe)) { + throw UserCancelledException(); + } + _log.error('Failed to add account.', pe); + rethrow; + } } @override @@ -222,11 +236,13 @@ class _AndroidCredentialListNotifier extends OathCredentialListNotifier { } return renamedCredential; - } on PlatformException catch (e) { - _log.debug('Failed to execute renameOathCredential: ${e.message}'); + } on PlatformException catch (pe) { + _log.debug('Failed to execute renameOathCredential: ${pe.message}'); + if (UserCancelledException.isCancellation(pe)) { + throw UserCancelledException(); + } + rethrow; } - - return credential; } @override @@ -237,8 +253,12 @@ class _AndroidCredentialListNotifier extends OathCredentialListNotifier { if (mounted) { state = state!.toList()..removeWhere((e) => e.credential == credential); } - } catch (e) { - _log.debug('Call to delete credential failed: $e'); + } on PlatformException catch (e) { + _log.debug('Received exception: $e'); + if (UserCancelledException.isCancellation(e)) { + throw UserCancelledException(); + } + rethrow; } } diff --git a/lib/app/views/user_interaction.dart b/lib/app/views/user_interaction.dart index 8006c03e..5ba5de88 100755 --- a/lib/app/views/user_interaction.dart +++ b/lib/app/views/user_interaction.dart @@ -99,12 +99,15 @@ UserInteractionController promptUserInteraction( Widget? icon, void Function()? onCancel, }) { + var wasPopped = false; final controller = _UserInteractionController( title: title, description: description, icon: icon, onClosed: () { - Navigator.of(context).pop(); + if (!wasPopped) { + Navigator.of(context).pop(); + } }, ); showDialog( @@ -114,6 +117,7 @@ UserInteractionController promptUserInteraction( onWillPop: () async { if (onCancel != null) { onCancel(); + wasPopped = true; return true; } else { return false; diff --git a/lib/oath/views/account_mixin.dart b/lib/oath/views/account_mixin.dart index 86b8821d..f8a2ced9 100755 --- a/lib/oath/views/account_mixin.dart +++ b/lib/oath/views/account_mixin.dart @@ -8,6 +8,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../../app/message.dart'; import '../../app/models.dart'; import '../../app/state.dart'; +import '../../user_cancelled_exception.dart'; import '../../widgets/circle_timer.dart'; import '../../widgets/custom_icons.dart'; import '../models.dart'; @@ -122,8 +123,12 @@ mixin AccountMixin { text: 'Calculate', icon: const Icon(Icons.refresh), action: ready - ? (context) { - calculateCode(context, ref); + ? (context) async { + try { + await calculateCode(context, ref); + } on UserCancelledException catch (_) { + // ignored + } } : null, ), diff --git a/lib/oath/views/account_view.dart b/lib/oath/views/account_view.dart index 68bb9a23..b2ce94ce 100755 --- a/lib/oath/views/account_view.dart +++ b/lib/oath/views/account_view.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:yubico_authenticator/user_cancelled_exception.dart'; import 'package:yubico_authenticator/app/state.dart'; import '../models.dart'; @@ -103,17 +104,21 @@ class AccountView extends ConsumerWidget with AccountMixin { ); }, onLongPress: () async { - if (calculateReady) { - await calculateCode( - context, - ref, + try { + if (calculateReady) { + await calculateCode( + context, + ref, + ); + } + await ref.read(withContextProvider)( + (context) async { + copyToClipboard(context, ref); + }, ); + } on UserCancelledException catch (_) { + // ignored } - await ref.read(withContextProvider)( - (context) async { - copyToClipboard(context, ref); - }, - ); }, leading: showAvatar ? CircleAvatar( diff --git a/lib/oath/views/add_account_page.dart b/lib/oath/views/add_account_page.dart index d322de63..26e35b18 100755 --- a/lib/oath/views/add_account_page.dart +++ b/lib/oath/views/add_account_page.dart @@ -5,6 +5,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:logging/logging.dart'; +import 'package:yubico_authenticator/user_cancelled_exception.dart'; import 'package:yubico_authenticator/app/logging.dart'; import '../../app/message.dart'; @@ -164,11 +165,13 @@ class _OathAddAccountPageState extends ConsumerState { try { await ref .read(credentialListProvider(widget.devicePath) - .notifier) + .notifier) .addAccount(cred.toUri(), requireTouch: _touch); if (!mounted) return; Navigator.of(context).pop(); showMessage(context, 'Account added'); + } on UserCancelledException catch (_) { + // ignored } catch (e) { _log.error('Failed to add account', e); showMessage(context, 'Failed adding account'); diff --git a/lib/oath/views/delete_account_dialog.dart b/lib/oath/views/delete_account_dialog.dart index dfc45574..5c3e58a2 100755 --- a/lib/oath/views/delete_account_dialog.dart +++ b/lib/oath/views/delete_account_dialog.dart @@ -1,5 +1,8 @@ import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:yubico_authenticator/user_cancelled_exception.dart'; +import 'package:yubico_authenticator/android/oath/state.dart'; import '../../app/message.dart'; import '../../widgets/responsive_dialog.dart'; @@ -29,15 +32,19 @@ class DeleteAccountDialog extends ConsumerWidget { actions: [ TextButton( onPressed: () async { - await ref - .read(credentialListProvider(device.path).notifier) - .deleteAccount(credential); - await ref.read(withContextProvider)( - (context) async { - Navigator.of(context).pop(); - showMessage(context, 'Account deleted'); - }, - ); + try { + await ref + .read(credentialListProvider(device.path).notifier) + .deleteAccount(credential); + await ref.read(withContextProvider)( + (context) async { + Navigator.of(context).pop(); + showMessage(context, 'Account deleted'); + }, + ); + } on UserCancelledException catch (_) { + // ignored + } }, child: const Text('Delete'), ), diff --git a/lib/oath/views/rename_account_dialog.dart b/lib/oath/views/rename_account_dialog.dart index 3c2185de..14d79a68 100755 --- a/lib/oath/views/rename_account_dialog.dart +++ b/lib/oath/views/rename_account_dialog.dart @@ -1,12 +1,13 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import '../../user_cancelled_exception.dart'; import '../../app/message.dart'; +import '../../app/models.dart'; +import '../../app/state.dart'; import '../../widgets/responsive_dialog.dart'; import '../models.dart'; import '../state.dart'; -import '../../app/models.dart'; -import '../../app/state.dart'; import 'utils.dart'; class RenameAccountDialog extends ConsumerStatefulWidget { @@ -59,13 +60,18 @@ class _RenameAccountDialogState extends ConsumerState { TextButton( onPressed: isValid ? () async { - final renamed = await ref - .read(credentialListProvider(widget.device.path).notifier) - .renameAccount(credential, - _issuer.isNotEmpty ? _issuer : null, _account); - if (!mounted) return; - Navigator.of(context).pop(renamed); - showMessage(context, 'Account renamed'); + try { + final renamed = await ref + .read( + credentialListProvider(widget.device.path).notifier) + .renameAccount(credential, + _issuer.isNotEmpty ? _issuer : null, _account); + if (!mounted) return; + Navigator.of(context).pop(renamed); + showMessage(context, 'Account renamed'); + } on UserCancelledException catch (_) { + // ignored + } } : null, child: const Text('Save'), diff --git a/lib/user_cancelled_exception.dart b/lib/user_cancelled_exception.dart new file mode 100644 index 00000000..e584e91d --- /dev/null +++ b/lib/user_cancelled_exception.dart @@ -0,0 +1,9 @@ +import 'package:flutter/services.dart'; + +class UserCancelledException implements Exception { + UserCancelledException(); + + static isCancellation(PlatformException pe) => + pe.code == 'UserCancelledException'; + +}