diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index c66a2d48..73a39185 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -17,6 +17,7 @@ "oath_add_account": "Add account", "oath_save": "Save", "oath_no_qr_code": "No QR code found", + "oath_duplicate_name": "This name already exists for the Issuer", "oath_issuer_optional": "Issuer (optional)", "oath_account_name": "Account name", "oath_secret_key": "Secret key", diff --git a/lib/oath/state.dart b/lib/oath/state.dart index d6ca7ca2..a3f23a48 100755 --- a/lib/oath/state.dart +++ b/lib/oath/state.dart @@ -150,6 +150,13 @@ class FavoritesNotifier extends StateNotifier> { } _prefs.setStringList(_key, state); } + + renameCredential(String oldCredentialId, String newCredentialId) { + if (state.contains(oldCredentialId)) { + state = [newCredentialId, ...state.toList()..remove(oldCredentialId)]; + _prefs.setStringList(_key, state); + } + } } final filteredCredentialsProvider = StateNotifierProvider.autoDispose diff --git a/lib/oath/views/account_mixin.dart b/lib/oath/views/account_mixin.dart index 9cf29204..221d1bee 100755 --- a/lib/oath/views/account_mixin.dart +++ b/lib/oath/views/account_mixin.dart @@ -80,9 +80,10 @@ mixin AccountMixin { Future renameCredential( BuildContext context, WidgetRef ref) async { final node = ref.read(currentDeviceProvider)!; + final credentials = ref.read(credentialsProvider); return await showBlurDialog( context: context, - builder: (context) => RenameAccountDialog(node, credential), + builder: (context) => RenameAccountDialog(node, credential, credentials), ); } diff --git a/lib/oath/views/add_account_page.dart b/lib/oath/views/add_account_page.dart index 262b1db7..e03cc47a 100755 --- a/lib/oath/views/add_account_page.dart +++ b/lib/oath/views/add_account_page.dart @@ -31,9 +31,15 @@ enum _QrScanState { none, scanning, success, failed } class OathAddAccountPage extends ConsumerStatefulWidget { final DevicePath devicePath; final OathState state; + final List? credentials; final bool openQrScanner; - const OathAddAccountPage(this.devicePath, this.state, - {super.key, required this.openQrScanner}); + const OathAddAccountPage( + this.devicePath, + this.state, { + super.key, + required this.openQrScanner, + required this.credentials, + }); @override ConsumerState createState() => @@ -55,6 +61,15 @@ class _OathAddAccountPageState extends ConsumerState { List _periodValues = [20, 30, 45, 60]; List _digitsValues = [6, 8]; + @override + void dispose() { + _issuerController.dispose(); + _accountController.dispose(); + _secretController.dispose(); + _periodController.dispose(); + super.dispose(); + } + _scanQrCode(QrScanner qrScanner) async { try { setState(() { @@ -102,8 +117,8 @@ class _OathAddAccountPageState extends ConsumerState { _loadCredentialData(CredentialData data) { setState(() { - _issuerController.text = data.issuer ?? ''; - _accountController.text = data.name; + _issuerController.text = data.issuer?.trim() ?? ''; + _accountController.text = data.name.trim(); _secretController.text = data.secret; _oathType = data.oathType; _hashAlgorithm = data.hashAlgorithm; @@ -134,16 +149,26 @@ class _OathAddAccountPageState extends ConsumerState { final remaining = getRemainingKeySpace( oathType: _oathType, period: period, - issuer: _issuerController.text, - name: _accountController.text, + issuer: _issuerController.text.trim(), + name: _accountController.text.trim(), ); final issuerRemaining = remaining.first; final nameRemaining = remaining.second; final secret = _secretController.text.replaceAll(' ', ''); final secretLengthValid = secret.length * 5 % 8 < 5; - final isValid = _accountController.text.isNotEmpty && + + // is this credentials name/issuer pair different from all other? + final isUnique = widget.credentials + ?.where((element) => + element.name == _accountController.text.trim() && + (element.issuer ?? '') == _issuerController.text.trim()) + .isEmpty ?? + false; + + final isValid = _accountController.text.trim().isNotEmpty && secret.isNotEmpty && + isUnique && issuerRemaining >= -1 && nameRemaining >= 0 && period > 0; @@ -152,11 +177,11 @@ class _OathAddAccountPageState extends ConsumerState { void submit() async { if (secretLengthValid) { - final issuer = _issuerController.text; + final issuer = _issuerController.text.trim(); final cred = CredentialData( issuer: issuer.isEmpty ? null : issuer, - name: _accountController.text, + name: _accountController.text.trim(), secret: secret, oathType: _oathType, hashAlgorithm: _hashAlgorithm, @@ -230,11 +255,11 @@ class _OathAddAccountPageState extends ConsumerState { enabled: issuerRemaining > 0, maxLength: max(issuerRemaining, 1), inputFormatters: [limitBytesLength(issuerRemaining)], - buildCounter: buildByteCounterFor(_issuerController.text), + buildCounter: buildByteCounterFor(_issuerController.text.trim()), decoration: InputDecoration( border: const OutlineInputBorder(), labelText: AppLocalizations.of(context)!.oath_issuer_optional, - helperText: '', // Prevents dialog resizing when enabled = false + helperText: '', // Prevents dialog resizing when disabled prefixIcon: const Icon(Icons.business_outlined), ), textInputAction: TextInputAction.next, @@ -251,13 +276,16 @@ class _OathAddAccountPageState extends ConsumerState { key: const Key('name'), controller: _accountController, maxLength: max(nameRemaining, 1), - buildCounter: buildByteCounterFor(_accountController.text), + buildCounter: buildByteCounterFor(_accountController.text.trim()), inputFormatters: [limitBytesLength(nameRemaining)], decoration: InputDecoration( border: const OutlineInputBorder(), - labelText: AppLocalizations.of(context)!.oath_account_name, - helperText: '', // Prevents dialog resizing when enabled = false prefixIcon: const Icon(Icons.person_outline), + labelText: AppLocalizations.of(context)!.oath_account_name, + helperText: '', // Prevents dialog resizing when disabled + errorText: isUnique + ? null + : AppLocalizations.of(context)!.oath_duplicate_name, ), textInputAction: TextInputAction.next, onChanged: (value) { diff --git a/lib/oath/views/oath_screen.dart b/lib/oath/views/oath_screen.dart index 7d255041..8d330270 100755 --- a/lib/oath/views/oath_screen.dart +++ b/lib/oath/views/oath_screen.dart @@ -124,8 +124,7 @@ class _UnlockedViewState extends ConsumerState<_UnlockedView> { header: 'No accounts', keyActions: _buildActions( context, - used: 0, - capacity: widget.oathState.version.isAtLeast(4) ? 32 : null, + credentials: null, ), ); } @@ -176,16 +175,19 @@ class _UnlockedViewState extends ConsumerState<_UnlockedView> { ), keyActions: _buildActions( context, - used: credentials?.length ?? 0, - capacity: widget.oathState.version.isAtLeast(4) ? 32 : null, + credentials: credentials, ), child: AccountList(widget.devicePath, widget.oathState), ), ); } - List _buildActions(BuildContext context, - {required int used, int? capacity}) { + List _buildActions( + BuildContext context, { + required List? credentials, + }) { + final used = credentials?.length ?? 0; + final capacity = widget.oathState.version.isAtLeast(4) ? 32 : null; return [ buildMenuItem( title: const Text('Add account'), @@ -198,6 +200,7 @@ class _UnlockedViewState extends ConsumerState<_UnlockedView> { builder: (context) => OathAddAccountPage( widget.devicePath, widget.oathState, + credentials: credentials, openQrScanner: Platform.isAndroid, ), ); diff --git a/lib/oath/views/rename_account_dialog.dart b/lib/oath/views/rename_account_dialog.dart index 4cda322e..1a83e04b 100755 --- a/lib/oath/views/rename_account_dialog.dart +++ b/lib/oath/views/rename_account_dialog.dart @@ -2,10 +2,10 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:logging/logging.dart'; -import '../../cancellation_exception.dart'; import '../../app/logging.dart'; import '../../app/message.dart'; import '../../app/models.dart'; +import '../../cancellation_exception.dart'; import '../../desktop/models.dart'; import '../../widgets/responsive_dialog.dart'; import '../../widgets/utf8_utils.dart'; @@ -18,7 +18,10 @@ final _log = Logger('oath.view.rename_account_dialog'); class RenameAccountDialog extends ConsumerStatefulWidget { final DeviceNode device; final OathCredential credential; - const RenameAccountDialog(this.device, this.credential, {super.key}); + final List? credentials; + + const RenameAccountDialog(this.device, this.credential, this.credentials, + {super.key}); @override ConsumerState createState() => @@ -32,8 +35,43 @@ class _RenameAccountDialogState extends ConsumerState { @override void initState() { super.initState(); - _issuer = widget.credential.issuer ?? ''; - _account = widget.credential.name; + _issuer = widget.credential.issuer?.trim() ?? ''; + _account = widget.credential.name.trim(); + } + + void _submit() async { + try { + // Rename credentials + final renamed = await ref + .read(credentialListProvider(widget.device.path).notifier) + .renameAccount( + widget.credential, _issuer.isNotEmpty ? _issuer : null, _account); + + // Update favorite + ref + .read(favoritesProvider.notifier) + .renameCredential(widget.credential.id, renamed.id); + + if (!mounted) return; + Navigator.of(context).pop(renamed); + showMessage(context, 'Account renamed'); + } on CancellationException catch (_) { + // ignored + } catch (e) { + _log.error('Failed to add account', e); + final String errorMessage; + // TODO: Make this cleaner than importing desktop specific RpcError. + if (e is RpcError) { + errorMessage = e.message; + } else { + errorMessage = e.toString(); + } + showMessage( + context, + 'Failed adding account: $errorMessage', + duration: const Duration(seconds: 4), + ); + } } @override @@ -52,42 +90,31 @@ class _RenameAccountDialogState extends ConsumerState { ); final issuerRemaining = remaining.first; final nameRemaining = remaining.second; - final isValid = _account.isNotEmpty; + + // is this credentials name/issuer pair different from all other? + final isUnique = widget.credentials + ?.where((element) => + element != credential && + element.name == _account && + (element.issuer ?? '') == _issuer) + .isEmpty ?? + false; + + // is this credential name/issuer of valid format + final isValidFormat = _account.isNotEmpty; + + // are the name/issuer values different from original + final didChange = (widget.credential.issuer ?? '') != _issuer || + widget.credential.name != _account; + + // can we rename with the new values + final isValid = isUnique && isValidFormat; return ResponsiveDialog( title: const Text('Rename account'), actions: [ TextButton( - onPressed: isValid - ? () async { - 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 CancellationException catch (_) { - // ignored - } catch (e) { - _log.error('Failed to add account', e); - final String errorMessage; - // TODO: Make this cleaner than importing desktop specific RpcError. - if (e is RpcError) { - errorMessage = e.message; - } else { - errorMessage = e.toString(); - } - showMessage( - context, - 'Failed adding account: $errorMessage', - duration: const Duration(seconds: 4), - ); - } - } - : null, + onPressed: didChange && isValid ? _submit : null, child: const Text('Save'), ), ], @@ -106,7 +133,7 @@ class _RenameAccountDialogState extends ConsumerState { decoration: const InputDecoration( border: OutlineInputBorder(), labelText: 'Issuer (optional)', - helperText: '', // Prevents dialog resizing when enabled = false + helperText: '', // Prevents dialog resizing when disabled prefixIcon: Icon(Icons.business_outlined), ), textInputAction: TextInputAction.next, @@ -124,8 +151,12 @@ class _RenameAccountDialogState extends ConsumerState { decoration: InputDecoration( border: const OutlineInputBorder(), labelText: 'Account name', - helperText: '', // Prevents dialog resizing when enabled = false - errorText: isValid ? null : 'Your account must have a name', + helperText: '', // Prevents dialog resizing when disabled + errorText: !isValidFormat + ? 'Your account must have a name' + : !isUnique + ? 'This name already exists for the Issuer' + : null, prefixIcon: const Icon(Icons.people_alt_outlined), ), textInputAction: TextInputAction.done, @@ -134,6 +165,11 @@ class _RenameAccountDialogState extends ConsumerState { _account = value.trim(); }); }, + onFieldSubmitted: (_) { + if (didChange && isValid) { + _submit(); + } + }, ), ] .map((e) => Padding(