Improve PIN handling consistency between views

This commit is contained in:
Dain Nilsson 2024-03-06 21:16:17 +01:00
parent 0f76aa5b1d
commit 3245c0d637
No known key found for this signature in database
GPG Key ID: F04367096FBA95E8
8 changed files with 145 additions and 43 deletions

View File

@ -44,7 +44,8 @@ class FidoPinDialog extends ConsumerStatefulWidget {
}
class _FidoPinDialogState extends ConsumerState<FidoPinDialog> {
String _currentPin = '';
final _currentPinController = TextEditingController();
final _currentPinFocus = FocusNode();
String _newPin = '';
String _confirmPin = '';
String? _currentPinError;
@ -54,15 +55,24 @@ class _FidoPinDialogState extends ConsumerState<FidoPinDialog> {
bool _isObscureCurrent = true;
bool _isObscureNew = true;
bool _isObscureConfirm = true;
bool _isBlocked = false;
@override
void dispose() {
_currentPinController.dispose();
_currentPinFocus.dispose();
super.dispose();
}
@override
Widget build(BuildContext context) {
final l10n = AppLocalizations.of(context)!;
final hasPin = widget.state.hasPin;
final isValid = _newPin.isNotEmpty &&
_newPin == _confirmPin &&
(!hasPin || _currentPin.isNotEmpty);
final minPinLength = widget.state.minPinLength;
// N.B. current PIN may be shorter than minimum, if it was set before the minimum was increased
final currentPinLenOk = !hasPin || _currentPinController.text.length >= 4;
final newPinLenOk = _newPin.length >= minPinLength;
final isValid = currentPinLenOk && newPinLenOk && _newPin == _confirmPin;
return ResponsiveDialog(
title: Text(hasPin ? l10n.s_change_pin : l10n.s_set_pin),
@ -82,11 +92,13 @@ class _FidoPinDialogState extends ConsumerState<FidoPinDialog> {
Text(l10n.p_enter_current_pin_or_reset_no_puk),
AppTextFormField(
key: currentPin,
initialValue: _currentPin,
controller: _currentPinController,
focusNode: _currentPinFocus,
autofocus: true,
obscureText: _isObscureCurrent,
autofillHints: const [AutofillHints.password],
decoration: AppInputDecoration(
enabled: !_isBlocked,
border: const OutlineInputBorder(),
labelText: l10n.s_current_pin,
errorText: _currentIsWrong ? _currentPinError : null,
@ -108,7 +120,6 @@ class _FidoPinDialogState extends ConsumerState<FidoPinDialog> {
onChanged: (value) {
setState(() {
_currentIsWrong = false;
_currentPin = value;
});
},
),
@ -124,7 +135,7 @@ class _FidoPinDialogState extends ConsumerState<FidoPinDialog> {
decoration: AppInputDecoration(
border: const OutlineInputBorder(),
labelText: l10n.s_new_pin,
enabled: !hasPin || _currentPin.isNotEmpty,
enabled: !_isBlocked && currentPinLenOk,
errorText: _newIsWrong ? _newPinError : null,
errorMaxLines: 3,
prefixIcon: const Icon(Symbols.pin),
@ -168,8 +179,7 @@ class _FidoPinDialogState extends ConsumerState<FidoPinDialog> {
tooltip:
_isObscureConfirm ? l10n.s_show_pin : l10n.s_hide_pin,
),
enabled:
(!hasPin || _currentPin.isNotEmpty) && _newPin.isNotEmpty,
enabled: !_isBlocked && currentPinLenOk && newPinLenOk,
),
onChanged: (value) {
setState(() {
@ -196,7 +206,10 @@ class _FidoPinDialogState extends ConsumerState<FidoPinDialog> {
void _submit() async {
final l10n = AppLocalizations.of(context)!;
final minPinLength = widget.state.minPinLength;
final oldPin = _currentPin.isNotEmpty ? _currentPin : null;
final oldPin = _currentPinController.text.isNotEmpty
? _currentPinController.text
: null;
// TODO: Remove this? It shouldn't happen...
if (_newPin.length < minPinLength) {
setState(() {
_newPinError = l10n.l_new_pin_len(minPinLength);
@ -213,9 +226,13 @@ class _FidoPinDialogState extends ConsumerState<FidoPinDialog> {
showMessage(context, l10n.s_pin_set);
}, failed: (retries, authBlocked) {
setState(() {
_currentPinController.selection = TextSelection(
baseOffset: 0, extentOffset: _currentPinController.text.length);
_currentPinFocus.requestFocus();
if (authBlocked) {
_currentPinError = l10n.l_pin_soft_locked;
_currentIsWrong = true;
_isBlocked = true;
} else {
_currentPinError = l10n.l_wrong_pin_attempts_remaining(retries);
_currentIsWrong = true;

View File

@ -37,11 +37,19 @@ class PinEntryForm extends ConsumerStatefulWidget {
class _PinEntryFormState extends ConsumerState<PinEntryForm> {
final _pinController = TextEditingController();
final _pinFocus = FocusNode();
bool _blocked = false;
int? _retries;
bool _pinIsWrong = false;
bool _isObscure = true;
@override
void dispose() {
_pinController.dispose();
_pinFocus.dispose();
super.dispose();
}
void _submit() async {
setState(() {
_pinIsWrong = false;
@ -51,8 +59,10 @@ class _PinEntryFormState extends ConsumerState<PinEntryForm> {
.read(fidoStateProvider(widget._deviceNode.path).notifier)
.unlock(_pinController.text);
result.whenOrNull(failed: (retries, authBlocked) {
_pinController.selection = TextSelection(
baseOffset: 0, extentOffset: _pinController.text.length);
_pinFocus.requestFocus();
setState(() {
_pinController.clear();
_pinIsWrong = true;
_retries = retries;
_blocked = authBlocked;
@ -92,6 +102,8 @@ class _PinEntryFormState extends ConsumerState<PinEntryForm> {
obscureText: _isObscure,
autofillHints: const [AutofillHints.password],
controller: _pinController,
focusNode: _pinFocus,
enabled: !_blocked && (_retries ?? 1) > 0,
decoration: AppInputDecoration(
border: const OutlineInputBorder(),
labelText: l10n.s_pin,
@ -137,7 +149,9 @@ class _PinEntryFormState extends ConsumerState<PinEntryForm> {
icon: const Icon(Symbols.lock_open),
label: Text(l10n.s_unlock),
onPressed:
_pinController.text.isNotEmpty && !_blocked ? _submit : null,
!_pinIsWrong && _pinController.text.length >= 4 && !_blocked
? _submit
: null,
),
),
],

View File

@ -41,7 +41,8 @@ class ManagePasswordDialog extends ConsumerStatefulWidget {
}
class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
String _currentPassword = '';
final _currentPasswordController = TextEditingController();
final _currentPasswordFocus = FocusNode();
String _newPassword = '';
String _confirmPassword = '';
bool _currentIsWrong = false;
@ -49,12 +50,19 @@ class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
bool _isObscureNew = true;
bool _isObscureConfirm = true;
@override
void dispose() {
_currentPasswordController.dispose();
_currentPasswordFocus.dispose();
super.dispose();
}
_submit() async {
FocusUtils.unfocus(context);
final result = await ref
.read(oathStateProvider(widget.path).notifier)
.setPassword(_currentPassword, _newPassword);
.setPassword(_currentPasswordController.text, _newPassword);
if (result) {
if (mounted) {
await ref.read(withContextProvider)((context) async {
@ -63,6 +71,9 @@ class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
});
}
} else {
_currentPasswordController.selection = TextSelection(
baseOffset: 0, extentOffset: _currentPasswordController.text.length);
_currentPasswordFocus.requestFocus();
setState(() {
_currentIsWrong = true;
});
@ -72,9 +83,10 @@ class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
@override
Widget build(BuildContext context) {
final l10n = AppLocalizations.of(context)!;
final isValid = _newPassword.isNotEmpty &&
final isValid = !_currentIsWrong &&
_newPassword.isNotEmpty &&
_newPassword == _confirmPassword &&
(!widget.state.hasKey || _currentPassword.isNotEmpty);
(!widget.state.hasKey || _currentPasswordController.text.isNotEmpty);
return ResponsiveDialog(
title: Text(
@ -98,6 +110,8 @@ class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
obscureText: _isObscureCurrent,
autofillHints: const [AutofillHints.password],
key: keys.currentPasswordField,
controller: _currentPasswordController,
focusNode: _currentPasswordFocus,
decoration: AppInputDecoration(
border: const OutlineInputBorder(),
labelText: l10n.s_current_password,
@ -121,7 +135,6 @@ class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
onChanged: (value) {
setState(() {
_currentIsWrong = false;
_currentPassword = value;
});
},
),
@ -131,11 +144,12 @@ class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
children: [
OutlinedButton(
key: keys.removePasswordButton,
onPressed: _currentPassword.isNotEmpty
onPressed: _currentPasswordController.text.isNotEmpty &&
!_currentIsWrong
? () async {
final result = await ref
.read(oathStateProvider(widget.path).notifier)
.unsetPassword(_currentPassword);
.unsetPassword(_currentPasswordController.text);
if (result) {
if (mounted) {
await ref.read(withContextProvider)(
@ -145,6 +159,12 @@ class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
});
}
} else {
_currentPasswordController.selection =
TextSelection(
baseOffset: 0,
extentOffset: _currentPasswordController
.text.length);
_currentPasswordFocus.requestFocus();
setState(() {
_currentIsWrong = true;
});
@ -193,7 +213,8 @@ class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
tooltip: _isObscureNew
? l10n.s_show_password
: l10n.s_hide_password),
enabled: !widget.state.hasKey || _currentPassword.isNotEmpty,
enabled: !widget.state.hasKey ||
_currentPasswordController.text.isNotEmpty,
),
textInputAction: TextInputAction.next,
onChanged: (value) {
@ -227,9 +248,9 @@ class _ManagePasswordDialogState extends ConsumerState<ManagePasswordDialog> {
tooltip: _isObscureConfirm
? l10n.s_show_password
: l10n.s_hide_password),
enabled:
(!widget.state.hasKey || _currentPassword.isNotEmpty) &&
_newPassword.isNotEmpty,
enabled: (!widget.state.hasKey ||
_currentPasswordController.text.isNotEmpty) &&
_newPassword.isNotEmpty,
),
textInputAction: TextInputAction.done,
onChanged: (value) {

View File

@ -38,6 +38,7 @@ class UnlockForm extends ConsumerStatefulWidget {
class _UnlockFormState extends ConsumerState<UnlockForm> {
final _passwordController = TextEditingController();
final _passwordFocus = FocusNode();
bool _remember = false;
bool _passwordIsWrong = false;
bool _isObscure = true;
@ -51,9 +52,11 @@ class _UnlockFormState extends ConsumerState<UnlockForm> {
.unlock(_passwordController.text, remember: _remember);
if (!mounted) return;
if (!success) {
_passwordController.selection = TextSelection(
baseOffset: 0, extentOffset: _passwordController.text.length);
_passwordFocus.requestFocus();
setState(() {
_passwordIsWrong = true;
_passwordController.clear();
});
} else if (_remember && !remembered) {
showMessage(context, AppLocalizations.of(context)!.l_remember_pw_failed);
@ -79,6 +82,7 @@ class _UnlockFormState extends ConsumerState<UnlockForm> {
child: AppTextField(
key: keys.passwordField,
controller: _passwordController,
focusNode: _passwordFocus,
autofocus: true,
obscureText: _isObscure,
autofillHints: const [AutofillHints.password],

View File

@ -44,10 +44,12 @@ class _AuthenticationDialogState extends ConsumerState<AuthenticationDialog> {
bool _keyIsWrong = false;
bool _keyFormatInvalid = false;
final _keyController = TextEditingController();
final _keyFocus = FocusNode();
@override
void dispose() {
_keyController.dispose();
_keyFocus.dispose();
super.dispose();
}
@ -65,7 +67,7 @@ class _AuthenticationDialogState extends ConsumerState<AuthenticationDialog> {
actions: [
TextButton(
key: keys.unlockButton,
onPressed: _keyController.text.length == keyLen
onPressed: !_keyIsWrong && _keyController.text.length == keyLen
? () async {
if (keyFormatInvalid) {
setState(() {
@ -81,6 +83,10 @@ class _AuthenticationDialogState extends ConsumerState<AuthenticationDialog> {
if (status) {
navigator.pop(true);
} else {
_keyController.selection = TextSelection(
baseOffset: 0,
extentOffset: _keyController.text.length);
_keyFocus.requestFocus();
setState(() {
_keyIsWrong = true;
});
@ -88,6 +94,10 @@ class _AuthenticationDialogState extends ConsumerState<AuthenticationDialog> {
} on CancellationException catch (_) {
navigator.pop(false);
} catch (_) {
_keyController.selection = TextSelection(
baseOffset: 0,
extentOffset: _keyController.text.length);
_keyFocus.requestFocus();
// TODO: More error cases
setState(() {
_keyIsWrong = true;
@ -109,6 +119,7 @@ class _AuthenticationDialogState extends ConsumerState<AuthenticationDialog> {
autofocus: true,
autofillHints: const [AutofillHints.password],
controller: _keyController,
focusNode: _keyFocus,
readOnly: _defaultKeyUsed,
maxLength: !_defaultKeyUsed ? keyLen : null,
decoration: AppInputDecoration(

View File

@ -57,6 +57,7 @@ class _ManageKeyDialogState extends ConsumerState<ManageKeyDialog> {
int _attemptsRemaining = -1;
late ManagementKeyType _keyType;
final _currentController = TextEditingController();
final _currentFocus = FocusNode();
final _keyController = TextEditingController();
bool _isObscure = true;
@ -84,6 +85,7 @@ class _ManageKeyDialogState extends ConsumerState<ManageKeyDialog> {
void dispose() {
_keyController.dispose();
_currentController.dispose();
_currentFocus.dispose();
super.dispose();
}
@ -103,6 +105,9 @@ class _ManageKeyDialogState extends ConsumerState<ManageKeyDialog> {
final status = (await notifier.verifyPin(_currentController.text)).when(
success: () => true,
failure: (attemptsRemaining) {
_currentController.selection = TextSelection(
baseOffset: 0, extentOffset: _currentController.text.length);
_currentFocus.requestFocus();
setState(() {
_attemptsRemaining = attemptsRemaining;
_currentIsWrong = true;
@ -115,6 +120,9 @@ class _ManageKeyDialogState extends ConsumerState<ManageKeyDialog> {
}
} else {
if (!await notifier.authenticate(_currentController.text)) {
_currentController.selection = TextSelection(
baseOffset: 0, extentOffset: _currentController.text.length);
_currentFocus.requestFocus();
setState(() {
_currentIsWrong = true;
});
@ -166,7 +174,8 @@ class _ManageKeyDialogState extends ConsumerState<ManageKeyDialog> {
title: Text(l10n.l_change_management_key),
actions: [
TextButton(
onPressed: currentLenOk && newLenOk ? _submit : null,
onPressed:
!_currentIsWrong && currentLenOk && newLenOk ? _submit : null,
key: keys.saveButton,
child: Text(l10n.s_save),
)
@ -185,6 +194,7 @@ class _ManageKeyDialogState extends ConsumerState<ManageKeyDialog> {
key: keys.pinPukField,
maxLength: 8,
controller: _currentController,
focusNode: _currentFocus,
readOnly: _defaultPinUsed,
decoration: AppInputDecoration(
border: const OutlineInputBorder(),
@ -223,6 +233,7 @@ class _ManageKeyDialogState extends ConsumerState<ManageKeyDialog> {
autofocus: !_defaultKeyUsed,
autofillHints: const [AutofillHints.password],
controller: _currentController,
focusNode: _currentFocus,
readOnly: _defaultKeyUsed,
maxLength: !_defaultKeyUsed ? currentType.keyLength * 2 : null,
decoration: AppInputDecoration(

View File

@ -44,20 +44,25 @@ class ManagePinPukDialog extends ConsumerStatefulWidget {
class _ManagePinPukDialogState extends ConsumerState<ManagePinPukDialog> {
final _currentPinController = TextEditingController();
final _currentPinFocus = FocusNode();
String _newPin = '';
String _confirmPin = '';
bool _pinIsBlocked = false;
bool _currentIsWrong = false;
int _attemptsRemaining = -1;
bool _isObscureCurrent = true;
bool _isObscureNew = true;
bool _isObscureConfirm = true;
late bool _defaultPinUsed;
late bool _defaultPukUsed;
late final bool _defaultPinUsed;
late final bool _defaultPukUsed;
late final int _minPinLen;
@override
void initState() {
super.initState();
// Old YubiKeys allowed a 4 digit PIN
_minPinLen = widget.pivState.version.isAtLeast(4, 3, 1) ? 6 : 4;
_defaultPinUsed =
widget.pivState.metadata?.pinMetadata.defaultValue ?? false;
_defaultPukUsed =
@ -73,6 +78,7 @@ class _ManagePinPukDialogState extends ConsumerState<ManagePinPukDialog> {
@override
void dispose() {
_currentPinController.dispose();
_currentPinFocus.dispose();
super.dispose();
}
@ -98,11 +104,16 @@ class _ManagePinPukDialogState extends ConsumerState<ManagePinPukDialog> {
_ => l10n.s_pin_set,
});
}, failure: (attemptsRemaining) {
_currentPinController.selection = TextSelection(
baseOffset: 0, extentOffset: _currentPinController.text.length);
_currentPinFocus.requestFocus();
setState(() {
_attemptsRemaining = attemptsRemaining;
_currentIsWrong = true;
if (_attemptsRemaining == 0) {
_pinIsBlocked = true;
}
});
_currentPinController.clear();
});
}
@ -110,8 +121,10 @@ class _ManagePinPukDialogState extends ConsumerState<ManagePinPukDialog> {
Widget build(BuildContext context) {
final l10n = AppLocalizations.of(context)!;
final currentPin = _currentPinController.text;
final isValid =
_newPin.isNotEmpty && _newPin == _confirmPin && currentPin.isNotEmpty;
final isValid = !_currentIsWrong &&
_newPin.isNotEmpty &&
_newPin == _confirmPin &&
currentPin.isNotEmpty;
final titleText = switch (widget.target) {
ManageTarget.pin => l10n.s_change_pin,
@ -138,7 +151,6 @@ class _ManagePinPukDialogState extends ConsumerState<ManagePinPukDialog> {
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: [
//TODO fix string
Text(widget.target == ManageTarget.pin
? l10n.p_enter_current_pin_or_reset
: l10n.p_enter_current_puk_or_reset),
@ -150,6 +162,8 @@ class _ManagePinPukDialogState extends ConsumerState<ManagePinPukDialog> {
key: keys.pinPukField,
readOnly: showDefaultPinUsed || showDefaultPukUsed,
controller: _currentPinController,
focusNode: _currentPinFocus,
enabled: !_pinIsBlocked,
decoration: AppInputDecoration(
border: const OutlineInputBorder(),
helperText: showDefaultPinUsed
@ -160,13 +174,17 @@ class _ManagePinPukDialogState extends ConsumerState<ManagePinPukDialog> {
labelText: widget.target == ManageTarget.pin
? l10n.s_current_pin
: l10n.s_current_puk,
errorText: _currentIsWrong
errorText: _pinIsBlocked
? (widget.target == ManageTarget.pin
? l10n
.l_wrong_pin_attempts_remaining(_attemptsRemaining)
: l10n
.l_wrong_puk_attempts_remaining(_attemptsRemaining))
: null,
? l10n.l_piv_pin_blocked
: l10n.l_piv_pin_puk_blocked)
: (_currentIsWrong
? (widget.target == ManageTarget.pin
? l10n.l_wrong_pin_attempts_remaining(
_attemptsRemaining)
: l10n.l_wrong_puk_attempts_remaining(
_attemptsRemaining))
: null),
errorMaxLines: 3,
prefixIcon: const Icon(Symbols.password),
suffixIcon: IconButton(
@ -217,8 +235,7 @@ class _ManagePinPukDialogState extends ConsumerState<ManagePinPukDialog> {
? (_isObscureNew ? l10n.s_show_pin : l10n.s_hide_pin)
: (_isObscureNew ? l10n.s_show_puk : l10n.s_hide_puk),
),
// Old YubiKeys allowed a 4 digit PIN
enabled: currentPin.length >= 4,
enabled: currentPin.length >= _minPinLen,
),
textInputAction: TextInputAction.next,
onChanged: (value) {
@ -256,7 +273,7 @@ class _ManagePinPukDialogState extends ConsumerState<ManagePinPukDialog> {
? (_isObscureConfirm ? l10n.s_show_pin : l10n.s_hide_pin)
: (_isObscureConfirm ? l10n.s_show_puk : l10n.s_hide_puk),
),
enabled: currentPin.length >= 4 && _newPin.length >= 6,
enabled: currentPin.length >= _minPinLen && _newPin.length >= 6,
),
textInputAction: TextInputAction.done,
onChanged: (value) {

View File

@ -37,6 +37,7 @@ class PinDialog extends ConsumerStatefulWidget {
class _PinDialogState extends ConsumerState<PinDialog> {
final _pinController = TextEditingController();
final _pinFocus = FocusNode();
bool _pinIsWrong = false;
int _attemptsRemaining = -1;
bool _isObscure = true;
@ -44,6 +45,7 @@ class _PinDialogState extends ConsumerState<PinDialog> {
@override
void dispose() {
_pinController.dispose();
_pinFocus.dispose();
super.dispose();
}
@ -58,8 +60,10 @@ class _PinDialogState extends ConsumerState<PinDialog> {
navigator.pop(true);
},
failure: (attemptsRemaining) {
_pinController.selection = TextSelection(
baseOffset: 0, extentOffset: _pinController.text.length);
_pinFocus.requestFocus();
setState(() {
_pinController.clear();
_attemptsRemaining = attemptsRemaining;
_pinIsWrong = true;
});
@ -73,12 +77,14 @@ class _PinDialogState extends ConsumerState<PinDialog> {
@override
Widget build(BuildContext context) {
final l10n = AppLocalizations.of(context)!;
final version = ref.watch(pivStateProvider(widget.devicePath)).valueOrNull;
final minPinLen = version?.version.isAtLeast(4, 3, 1) == true ? 6 : 4;
return ResponsiveDialog(
title: Text(l10n.s_pin_required),
actions: [
TextButton(
key: keys.unlockButton,
onPressed: _pinController.text.length >= 4 ? _submit : null,
onPressed: _pinController.text.length >= minPinLen ? _submit : null,
child: Text(l10n.s_unlock),
),
],
@ -95,6 +101,7 @@ class _PinDialogState extends ConsumerState<PinDialog> {
autofillHints: const [AutofillHints.password],
key: keys.managementKeyField,
controller: _pinController,
focusNode: _pinFocus,
decoration: AppInputDecoration(
border: const OutlineInputBorder(),
labelText: l10n.s_pin,