From 40335e66da49bce861d69352dad8b3863ad52e82 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Mon, 31 Jan 2022 11:02:34 +0100 Subject: [PATCH] Refactor common code between add and rename accounts. --- lib/core/models.dart | 5 + lib/core/models.freezed.dart | 144 ++++++++++++++++++++++ lib/oath/models.freezed.dart | 20 +-- lib/oath/models.g.dart | 10 +- lib/oath/views/add_account_page.dart | 33 +++-- lib/oath/views/rename_account_dialog.dart | 64 +++++----- lib/oath/views/utils.dart | 32 +++++ 7 files changed, 240 insertions(+), 68 deletions(-) create mode 100755 lib/oath/views/utils.dart diff --git a/lib/core/models.dart b/lib/core/models.dart index 0848d5f0..3d41c465 100644 --- a/lib/core/models.dart +++ b/lib/core/models.dart @@ -18,3 +18,8 @@ class Version with _$Version { return '$major.$minor.$patch'; } } + +@freezed +class Pair with _$Pair { + factory Pair(T1 first, T2 second) = _Pair; +} diff --git a/lib/core/models.freezed.dart b/lib/core/models.freezed.dart index 35563eb5..054dce00 100755 --- a/lib/core/models.freezed.dart +++ b/lib/core/models.freezed.dart @@ -168,3 +168,147 @@ abstract class _Version extends Version { _$VersionCopyWith<_Version> get copyWith => throw _privateConstructorUsedError; } + +/// @nodoc +class _$PairTearOff { + const _$PairTearOff(); + + _Pair call(T1 first, T2 second) { + return _Pair( + first, + second, + ); + } +} + +/// @nodoc +const $Pair = _$PairTearOff(); + +/// @nodoc +mixin _$Pair { + T1 get first => throw _privateConstructorUsedError; + T2 get second => throw _privateConstructorUsedError; + + @JsonKey(ignore: true) + $PairCopyWith> get copyWith => + throw _privateConstructorUsedError; +} + +/// @nodoc +abstract class $PairCopyWith { + factory $PairCopyWith(Pair value, $Res Function(Pair) then) = + _$PairCopyWithImpl; + $Res call({T1 first, T2 second}); +} + +/// @nodoc +class _$PairCopyWithImpl implements $PairCopyWith { + _$PairCopyWithImpl(this._value, this._then); + + final Pair _value; + // ignore: unused_field + final $Res Function(Pair) _then; + + @override + $Res call({ + Object? first = freezed, + Object? second = freezed, + }) { + return _then(_value.copyWith( + first: first == freezed + ? _value.first + : first // ignore: cast_nullable_to_non_nullable + as T1, + second: second == freezed + ? _value.second + : second // ignore: cast_nullable_to_non_nullable + as T2, + )); + } +} + +/// @nodoc +abstract class _$PairCopyWith + implements $PairCopyWith { + factory _$PairCopyWith( + _Pair value, $Res Function(_Pair) then) = + __$PairCopyWithImpl; + @override + $Res call({T1 first, T2 second}); +} + +/// @nodoc +class __$PairCopyWithImpl extends _$PairCopyWithImpl + implements _$PairCopyWith { + __$PairCopyWithImpl(_Pair _value, $Res Function(_Pair) _then) + : super(_value, (v) => _then(v as _Pair)); + + @override + _Pair get _value => super._value as _Pair; + + @override + $Res call({ + Object? first = freezed, + Object? second = freezed, + }) { + return _then(_Pair( + first == freezed + ? _value.first + : first // ignore: cast_nullable_to_non_nullable + as T1, + second == freezed + ? _value.second + : second // ignore: cast_nullable_to_non_nullable + as T2, + )); + } +} + +/// @nodoc + +class _$_Pair implements _Pair { + _$_Pair(this.first, this.second); + + @override + final T1 first; + @override + final T2 second; + + @override + String toString() { + return 'Pair<$T1, $T2>(first: $first, second: $second)'; + } + + @override + bool operator ==(dynamic other) { + return identical(this, other) || + (other.runtimeType == runtimeType && + other is _Pair && + const DeepCollectionEquality().equals(other.first, first) && + const DeepCollectionEquality().equals(other.second, second)); + } + + @override + int get hashCode => Object.hash( + runtimeType, + const DeepCollectionEquality().hash(first), + const DeepCollectionEquality().hash(second)); + + @JsonKey(ignore: true) + @override + _$PairCopyWith> get copyWith => + __$PairCopyWithImpl>(this, _$identity); +} + +abstract class _Pair implements Pair { + factory _Pair(T1 first, T2 second) = _$_Pair; + + @override + T1 get first; + @override + T2 get second; + @override + @JsonKey(ignore: true) + _$PairCopyWith> get copyWith => + throw _privateConstructorUsedError; +} diff --git a/lib/oath/models.freezed.dart b/lib/oath/models.freezed.dart index 53e03384..5e0ee949 100755 --- a/lib/oath/models.freezed.dart +++ b/lib/oath/models.freezed.dart @@ -837,11 +837,11 @@ class _$CredentialDataTearOff { {String? issuer, required String name, required String secret, - OathType oathType = OathType.totp, - HashAlgorithm hashAlgorithm = HashAlgorithm.sha1, - int digits = 6, - int period = 30, - int counter = 0}) { + OathType oathType = defaultOathType, + HashAlgorithm hashAlgorithm = defaultHashAlgorithm, + int digits = defaultDigits, + int period = defaultPeriod, + int counter = defaultCounter}) { return _CredentialData( issuer: issuer, name: name, @@ -1036,11 +1036,11 @@ class _$_CredentialData extends _CredentialData { {this.issuer, required this.name, required this.secret, - this.oathType = OathType.totp, - this.hashAlgorithm = HashAlgorithm.sha1, - this.digits = 6, - this.period = 30, - this.counter = 0}) + this.oathType = defaultOathType, + this.hashAlgorithm = defaultHashAlgorithm, + this.digits = defaultDigits, + this.period = defaultPeriod, + this.counter = defaultCounter}) : super._(); factory _$_CredentialData.fromJson(Map json) => diff --git a/lib/oath/models.g.dart b/lib/oath/models.g.dart index 99f1195d..f28c6cb8 100755 --- a/lib/oath/models.g.dart +++ b/lib/oath/models.g.dart @@ -65,13 +65,13 @@ _$_CredentialData _$$_CredentialDataFromJson(Map json) => name: json['name'] as String, secret: json['secret'] as String, oathType: $enumDecodeNullable(_$OathTypeEnumMap, json['oath_type']) ?? - OathType.totp, + defaultOathType, hashAlgorithm: $enumDecodeNullable(_$HashAlgorithmEnumMap, json['hash_algorithm']) ?? - HashAlgorithm.sha1, - digits: json['digits'] as int? ?? 6, - period: json['period'] as int? ?? 30, - counter: json['counter'] as int? ?? 0, + defaultHashAlgorithm, + digits: json['digits'] as int? ?? defaultDigits, + period: json['period'] as int? ?? defaultPeriod, + counter: json['counter'] as int? ?? defaultCounter, ); Map _$$_CredentialDataToJson(_$_CredentialData instance) => diff --git a/lib/oath/views/add_account_page.dart b/lib/oath/views/add_account_page.dart index 4216f61a..27a48e63 100755 --- a/lib/oath/views/add_account_page.dart +++ b/lib/oath/views/add_account_page.dart @@ -8,6 +8,7 @@ import 'package:yubico_authenticator/oath/models.dart'; import '../../app/state.dart'; import '../../app/models.dart'; import '../state.dart'; +import 'utils.dart'; final _secretFormatterPattern = RegExp('[abcdefghijklmnopqrstuvwxyz234567 ]', caseSensitive: false); @@ -33,17 +34,14 @@ class _AddAccountFormState extends State { @override Widget build(BuildContext context) { - int remaining = 64; // 64 bytes are shared between issuer and name. - if (_oathType == OathType.totp && _period != defaultPeriod) { - // Non-standard periods are stored as part of this data, as a "D/"- prefix. - remaining -= '$_period/'.length; - } - if (_issuer.isNotEmpty) { - // Issuer is separated from name with a ":", if present. - remaining -= 1; - } - final issuerRemaining = remaining - max(_account.length, 1); - final nameRemaining = remaining - _issuer.length; + final remaining = getRemainingKeySpace( + oathType: _oathType, + period: _period, + issuer: _issuer, + name: _account, + ); + final issuerRemaining = remaining.first; + final nameRemaining = remaining.second; final secretValid = _secret.length * 5 % 8 < 5; final isValid = @@ -57,7 +55,7 @@ class _AddAccountFormState extends State { children: [ TextField( enabled: issuerRemaining > 0, - maxLength: issuerRemaining > 0 ? issuerRemaining : null, + maxLength: max(issuerRemaining, 1), decoration: const InputDecoration( labelText: 'Issuer (optional)', helperText: @@ -65,13 +63,12 @@ class _AddAccountFormState extends State { ), onChanged: (value) { setState(() { - _issuer = value; + _issuer = value.trim(); }); }, ), - TextFormField( - enabled: nameRemaining > 0, - maxLength: nameRemaining > 0 ? nameRemaining : null, + TextField( + maxLength: nameRemaining, decoration: const InputDecoration( labelText: 'Account name', helperText: @@ -79,11 +76,11 @@ class _AddAccountFormState extends State { ), onChanged: (value) { setState(() { - _account = value; + _account = value.trim(); }); }, ), - TextFormField( + TextField( inputFormatters: [ FilteringTextInputFormatter.allow(_secretFormatterPattern) ], diff --git a/lib/oath/views/rename_account_dialog.dart b/lib/oath/views/rename_account_dialog.dart index 70f5790f..c8beedf6 100755 --- a/lib/oath/views/rename_account_dialog.dart +++ b/lib/oath/views/rename_account_dialog.dart @@ -1,5 +1,3 @@ -import 'dart:math'; - import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; @@ -7,6 +5,7 @@ import '../models.dart'; import '../state.dart'; import '../../app/models.dart'; import '../../app/state.dart'; +import 'utils.dart'; class RenameAccountDialog extends ConsumerStatefulWidget { final DeviceNode device; @@ -20,16 +19,15 @@ class RenameAccountDialog extends ConsumerStatefulWidget { } class _RenameAccountDialogState extends ConsumerState { - late TextEditingController _issuerController; - late TextEditingController _nameController; + late String _issuer; + late String _account; _RenameAccountDialogState(); @override void initState() { super.initState(); - _issuerController = - TextEditingController(text: widget.credential.issuer ?? ''); - _nameController = TextEditingController(text: widget.credential.name); + _issuer = widget.credential.issuer ?? ''; + _account = widget.credential.name; } @override @@ -45,20 +43,15 @@ class _RenameAccountDialogState extends ConsumerState { ? '${credential.issuer} (${credential.name})' : credential.name; - int remaining = 64; // 64 bytes are shared between issuer and name. - if (credential.oathType == OathType.totp && - credential.period != defaultPeriod) { - // Non-standard periods are stored as part of this data, as a "D/"- prefix. - remaining -= '${credential.period}/'.length; - } - if (_issuerController.text.isNotEmpty) { - // Issuer is separated from name with a ":", if present. - remaining -= 1; - } - final issuerRemaining = - remaining - max(_nameController.text.length, 1); - final nameRemaining = remaining - _issuerController.text.length; - final isValid = _nameController.text.trim().isNotEmpty; + final remaining = getRemainingKeySpace( + oathType: credential.oathType, + period: credential.period, + issuer: _issuer, + name: _account, + ); + final issuerRemaining = remaining.first; + final nameRemaining = remaining.second; + final isValid = _account.isNotEmpty; return AlertDialog( title: Text('Rename $label?'), @@ -67,29 +60,32 @@ class _RenameAccountDialogState extends ConsumerState { children: [ const Text( 'This will change how the account is displayed in the list.'), - TextField( - controller: _issuerController, + TextFormField( + initialValue: _issuer, enabled: issuerRemaining > 0, maxLength: issuerRemaining > 0 ? issuerRemaining : null, decoration: const InputDecoration( - labelText: 'Issuer', + labelText: 'Issuer (optional)', helperText: '', // Prevents dialog resizing when enabled = false ), onChanged: (value) { - setState(() {}); // Update maxLength + setState(() { + _issuer = value.trim(); + }); }, ), - TextField( - controller: _nameController, - enabled: nameRemaining > 0, - maxLength: nameRemaining > 0 ? nameRemaining : null, + TextFormField( + initialValue: _account, + maxLength: nameRemaining, decoration: InputDecoration( - labelText: 'Account name *', + labelText: 'Account name', helperText: '', // Prevents dialog resizing when enabled = false errorText: isValid ? null : 'Your account must have a name', ), onChanged: (value) { - setState(() {}); // Update maxLength, isValid + setState(() { + _account = value.trim(); + }); }, ), ], @@ -104,12 +100,10 @@ class _RenameAccountDialogState extends ConsumerState { ElevatedButton( onPressed: isValid ? () async { - final issuer = _issuerController.text.trim(); - final name = _nameController.text.trim(); await ref .read(credentialListProvider(widget.device.path).notifier) - .renameAccount( - credential, issuer.isNotEmpty ? issuer : null, name); + .renameAccount(credential, + _issuer.isNotEmpty ? _issuer : null, _account); Navigator.of(context).pop(); ScaffoldMessenger.of(context).showSnackBar( const SnackBar( diff --git a/lib/oath/views/utils.dart b/lib/oath/views/utils.dart new file mode 100755 index 00000000..96cb3b09 --- /dev/null +++ b/lib/oath/views/utils.dart @@ -0,0 +1,32 @@ +import 'dart:math'; + +import '../models.dart'; +import '../../core/models.dart'; + +/// Calculates the available space for issuer and account name. +/// +/// Returns a [Pair] of the space available for the issuer and account name, +/// respectively, based on the current state of the credential. +Pair getRemainingKeySpace( + {required OathType oathType, + required int period, + required String issuer, + required String name}) { + int remaining = 64; // The field is 64 bytes in total. + + if (oathType == OathType.totp && period != defaultPeriod) { + // Non-standard TOTP periods are stored as part of this data, as a "D/"- prefix. + remaining -= '$period/'.length; + } + int issuerSpace = issuer.length; + if (issuer.isNotEmpty) { + // Issuer is separated from name with a ":", if present. + issuerSpace += 1; + } + + return Pair( + // Always reserve at least one character for name + remaining - 1 - max(name.length, 1), + remaining - issuerSpace, + ); +}