Merge PR #198.

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",

lib/oath/state.dart

@@ -150,6 +150,13 @@ class FavoritesNotifier extends StateNotifier<List<String>> {
     }
     _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

lib/oath/views/account_mixin.dart

@@ -80,9 +80,10 @@ mixin AccountMixin {
   Future<OathCredential?> 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),
     );
   }
 

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<OathCredential>? 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<ConsumerStatefulWidget> createState() =>
@@ -55,6 +61,15 @@ class _OathAddAccountPageState extends ConsumerState<OathAddAccountPage> {
   List<int> _periodValues = [20, 30, 45, 60];
   List<int> _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<OathAddAccountPage> {
 
   _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<OathAddAccountPage> {
     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<OathAddAccountPage> {
 
     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<OathAddAccountPage> {
               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<OathAddAccountPage> {
               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) {

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<PopupMenuEntry> _buildActions(BuildContext context,
-      {required int used, int? capacity}) {
+  List<PopupMenuEntry> _buildActions(
+    BuildContext context, {
+    required List<OathCredential>? 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,
                   ),
                 );

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<OathCredential>? credentials;
+
+  const RenameAccountDialog(this.device, this.credential, this.credentials,
+      {super.key});
 
   @override
   ConsumerState<ConsumerStatefulWidget> createState() =>
@@ -32,8 +35,43 @@ class _RenameAccountDialogState extends ConsumerState<RenameAccountDialog> {
   @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<RenameAccountDialog> {
     );
     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<RenameAccountDialog> {
             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<RenameAccountDialog> {
             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<RenameAccountDialog> {
                 _account = value.trim();
               });
             },
+            onFieldSubmitted: (_) {
+              if (didChange && isValid) {
+                _submit();
+              }
+            },
           ),
         ]
             .map((e) => Padding(