From d47f8d7fe20b0f8aa460487702665439bb136907 Mon Sep 17 00:00:00 2001 From: Dain Nilsson Date: Wed, 23 Aug 2023 15:53:30 +0200 Subject: [PATCH] PIV: improve Import File dialog. --- helper/helper/piv.py | 50 +++++++++----- lib/l10n/app_en.arb | 15 +--- lib/piv/models.dart | 4 +- lib/piv/models.freezed.dart | 97 ++++++++++++++++---------- lib/piv/models.g.dart | 10 +-- lib/piv/views/actions.dart | 7 +- lib/piv/views/cert_info_view.dart | 99 +++++++++++++++++++++++++++ lib/piv/views/import_file_dialog.dart | 41 ++++++++++- lib/piv/views/slot_dialog.dart | 56 +-------------- 9 files changed, 245 insertions(+), 134 deletions(-) create mode 100644 lib/piv/views/cert_info_view.dart diff --git a/helper/helper/piv.py b/helper/helper/piv.py index 5d7390c1..2478aa78 100644 --- a/helper/helper/piv.py +++ b/helper/helper/piv.py @@ -240,11 +240,15 @@ class PivNode(RpcNode): password = params.pop("password", None) try: private_key, certs = _parse_file(data, password) + certificate = _choose_cert(certs) + return dict( status=True, password=password is not None, - private_key=bool(private_key), - certificates=len(certs), + key_type=KEY_TYPE.from_public_key(private_key.public_key()) + if private_key + else None, + cert_info=_get_cert_info(certificate), ) except InvalidPasswordError: logger.debug("Invalid or missing password", exc_info=True) @@ -279,6 +283,29 @@ def _parse_file(data, password=None): return private_key, certs +def _choose_cert(certs): + if certs: + if len(certs) > 1: + leafs = get_leaf_certificates(certs) + return leafs[0] + else: + return certs[0] + return None + + +def _get_cert_info(cert): + if cert is None: + return None + return dict( + subject=cert.subject.rfc4514_string(), + issuer=cert.issuer.rfc4514_string(), + serial=hex(cert.serial_number)[2:], + not_valid_before=cert.not_valid_before.isoformat(), + not_valid_after=cert.not_valid_after.isoformat(), + fingerprint=cert.fingerprint(hashes.SHA256()), + ) + + class SlotsNode(RpcNode): def __init__(self, session): super().__init__() @@ -314,16 +341,7 @@ class SlotsNode(RpcNode): slot=int(slot), name=slot.name, has_key=metadata is not None if self._has_metadata else None, - cert_info=dict( - subject=cert.subject.rfc4514_string(), - issuer=cert.issuer.rfc4514_string(), - serial=hex(cert.serial_number)[2:], - not_valid_before=cert.not_valid_before.isoformat(), - not_valid_after=cert.not_valid_after.isoformat(), - fingerprint=cert.fingerprint(hashes.SHA256()), - ) - if cert - else None, + cert_info=_get_cert_info(cert), ) for slot, (metadata, cert) in self._slots.items() } @@ -390,12 +408,8 @@ class SlotNode(RpcNode): except (ApduError, BadResponseError): pass - if certs: - if len(certs) > 1: - leafs = get_leaf_certificates(certs) - certificate = leafs[0] - else: - certificate = certs[0] + certificate = _choose_cert(certs) + if certificate: self.session.put_certificate(self.slot, certificate) self.session.put_object(OBJECT_ID.CHUID, generate_chuid()) self.certificate = certificate diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index 03b20f80..e8cae693 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -43,20 +43,9 @@ "label": {} } }, - "l_bullet": "• {item}", - "@l_bullet" : { - "placeholders": { - "item": {} - } - }, - "s_definition": "{item}:", - "@s_definition" : { - "placeholders": { - "item": {} - } - }, "s_about": "About", + "s_algorithm": "Algorithm", "s_appearance": "Appearance", "s_authenticator": "Authenticator", "s_actions": "Actions", @@ -460,7 +449,7 @@ }, "l_certificate_deleted": "Certificate deleted", "p_password_protected_file": "The selected file is password protected. Enter the password to proceed.", - "p_import_items_desc": "The following items will be imported into PIV slot {slot}.", + "p_import_items_desc": "The following item(s) will be imported into PIV slot {slot}.", "@p_import_items_desc" : { "placeholders": { "slot": {} diff --git a/lib/piv/models.dart b/lib/piv/models.dart index d42ab88f..d54eaeee 100644 --- a/lib/piv/models.dart +++ b/lib/piv/models.dart @@ -266,8 +266,8 @@ class PivSlot with _$PivSlot { class PivExamineResult with _$PivExamineResult { factory PivExamineResult.result({ required bool password, - required bool privateKey, - required int certificates, + required KeyType? keyType, + required CertInfo? certInfo, }) = _ExamineResult; factory PivExamineResult.invalidPassword() = _InvalidPassword; diff --git a/lib/piv/models.freezed.dart b/lib/piv/models.freezed.dart index 8a1637f5..9352b2d4 100644 --- a/lib/piv/models.freezed.dart +++ b/lib/piv/models.freezed.dart @@ -1860,20 +1860,23 @@ PivExamineResult _$PivExamineResultFromJson(Map json) { mixin _$PivExamineResult { @optionalTypeArgs TResult when({ - required TResult Function(bool password, bool privateKey, int certificates) + required TResult Function( + bool password, KeyType? keyType, CertInfo? certInfo) result, required TResult Function() invalidPassword, }) => throw _privateConstructorUsedError; @optionalTypeArgs TResult? whenOrNull({ - TResult? Function(bool password, bool privateKey, int certificates)? result, + TResult? Function(bool password, KeyType? keyType, CertInfo? certInfo)? + result, TResult? Function()? invalidPassword, }) => throw _privateConstructorUsedError; @optionalTypeArgs TResult maybeWhen({ - TResult Function(bool password, bool privateKey, int certificates)? result, + TResult Function(bool password, KeyType? keyType, CertInfo? certInfo)? + result, TResult Function()? invalidPassword, required TResult orElse(), }) => @@ -1924,7 +1927,9 @@ abstract class _$$_ExamineResultCopyWith<$Res> { _$_ExamineResult value, $Res Function(_$_ExamineResult) then) = __$$_ExamineResultCopyWithImpl<$Res>; @useResult - $Res call({bool password, bool privateKey, int certificates}); + $Res call({bool password, KeyType? keyType, CertInfo? certInfo}); + + $CertInfoCopyWith<$Res>? get certInfo; } /// @nodoc @@ -1939,24 +1944,36 @@ class __$$_ExamineResultCopyWithImpl<$Res> @override $Res call({ Object? password = null, - Object? privateKey = null, - Object? certificates = null, + Object? keyType = freezed, + Object? certInfo = freezed, }) { return _then(_$_ExamineResult( password: null == password ? _value.password : password // ignore: cast_nullable_to_non_nullable as bool, - privateKey: null == privateKey - ? _value.privateKey - : privateKey // ignore: cast_nullable_to_non_nullable - as bool, - certificates: null == certificates - ? _value.certificates - : certificates // ignore: cast_nullable_to_non_nullable - as int, + keyType: freezed == keyType + ? _value.keyType + : keyType // ignore: cast_nullable_to_non_nullable + as KeyType?, + certInfo: freezed == certInfo + ? _value.certInfo + : certInfo // ignore: cast_nullable_to_non_nullable + as CertInfo?, )); } + + @override + @pragma('vm:prefer-inline') + $CertInfoCopyWith<$Res>? get certInfo { + if (_value.certInfo == null) { + return null; + } + + return $CertInfoCopyWith<$Res>(_value.certInfo!, (value) { + return _then(_value.copyWith(certInfo: value)); + }); + } } /// @nodoc @@ -1964,8 +1981,8 @@ class __$$_ExamineResultCopyWithImpl<$Res> class _$_ExamineResult implements _ExamineResult { _$_ExamineResult( {required this.password, - required this.privateKey, - required this.certificates, + required this.keyType, + required this.certInfo, final String? $type}) : $type = $type ?? 'result'; @@ -1975,16 +1992,16 @@ class _$_ExamineResult implements _ExamineResult { @override final bool password; @override - final bool privateKey; + final KeyType? keyType; @override - final int certificates; + final CertInfo? certInfo; @JsonKey(name: 'runtimeType') final String $type; @override String toString() { - return 'PivExamineResult.result(password: $password, privateKey: $privateKey, certificates: $certificates)'; + return 'PivExamineResult.result(password: $password, keyType: $keyType, certInfo: $certInfo)'; } @override @@ -1994,16 +2011,14 @@ class _$_ExamineResult implements _ExamineResult { other is _$_ExamineResult && (identical(other.password, password) || other.password == password) && - (identical(other.privateKey, privateKey) || - other.privateKey == privateKey) && - (identical(other.certificates, certificates) || - other.certificates == certificates)); + (identical(other.keyType, keyType) || other.keyType == keyType) && + (identical(other.certInfo, certInfo) || + other.certInfo == certInfo)); } @JsonKey(ignore: true) @override - int get hashCode => - Object.hash(runtimeType, password, privateKey, certificates); + int get hashCode => Object.hash(runtimeType, password, keyType, certInfo); @JsonKey(ignore: true) @override @@ -2014,31 +2029,34 @@ class _$_ExamineResult implements _ExamineResult { @override @optionalTypeArgs TResult when({ - required TResult Function(bool password, bool privateKey, int certificates) + required TResult Function( + bool password, KeyType? keyType, CertInfo? certInfo) result, required TResult Function() invalidPassword, }) { - return result(password, privateKey, certificates); + return result(password, keyType, certInfo); } @override @optionalTypeArgs TResult? whenOrNull({ - TResult? Function(bool password, bool privateKey, int certificates)? result, + TResult? Function(bool password, KeyType? keyType, CertInfo? certInfo)? + result, TResult? Function()? invalidPassword, }) { - return result?.call(password, privateKey, certificates); + return result?.call(password, keyType, certInfo); } @override @optionalTypeArgs TResult maybeWhen({ - TResult Function(bool password, bool privateKey, int certificates)? result, + TResult Function(bool password, KeyType? keyType, CertInfo? certInfo)? + result, TResult Function()? invalidPassword, required TResult orElse(), }) { if (result != null) { - return result(password, privateKey, certificates); + return result(password, keyType, certInfo); } return orElse(); } @@ -2085,15 +2103,15 @@ class _$_ExamineResult implements _ExamineResult { abstract class _ExamineResult implements PivExamineResult { factory _ExamineResult( {required final bool password, - required final bool privateKey, - required final int certificates}) = _$_ExamineResult; + required final KeyType? keyType, + required final CertInfo? certInfo}) = _$_ExamineResult; factory _ExamineResult.fromJson(Map json) = _$_ExamineResult.fromJson; bool get password; - bool get privateKey; - int get certificates; + KeyType? get keyType; + CertInfo? get certInfo; @JsonKey(ignore: true) _$$_ExamineResultCopyWith<_$_ExamineResult> get copyWith => throw _privateConstructorUsedError; @@ -2145,7 +2163,8 @@ class _$_InvalidPassword implements _InvalidPassword { @override @optionalTypeArgs TResult when({ - required TResult Function(bool password, bool privateKey, int certificates) + required TResult Function( + bool password, KeyType? keyType, CertInfo? certInfo) result, required TResult Function() invalidPassword, }) { @@ -2155,7 +2174,8 @@ class _$_InvalidPassword implements _InvalidPassword { @override @optionalTypeArgs TResult? whenOrNull({ - TResult? Function(bool password, bool privateKey, int certificates)? result, + TResult? Function(bool password, KeyType? keyType, CertInfo? certInfo)? + result, TResult? Function()? invalidPassword, }) { return invalidPassword?.call(); @@ -2164,7 +2184,8 @@ class _$_InvalidPassword implements _InvalidPassword { @override @optionalTypeArgs TResult maybeWhen({ - TResult Function(bool password, bool privateKey, int certificates)? result, + TResult Function(bool password, KeyType? keyType, CertInfo? certInfo)? + result, TResult Function()? invalidPassword, required TResult orElse(), }) { diff --git a/lib/piv/models.g.dart b/lib/piv/models.g.dart index 109f280d..763d8393 100644 --- a/lib/piv/models.g.dart +++ b/lib/piv/models.g.dart @@ -168,16 +168,18 @@ const _$SlotIdEnumMap = { _$_ExamineResult _$$_ExamineResultFromJson(Map json) => _$_ExamineResult( password: json['password'] as bool, - privateKey: json['private_key'] as bool, - certificates: json['certificates'] as int, + keyType: $enumDecodeNullable(_$KeyTypeEnumMap, json['key_type']), + certInfo: json['cert_info'] == null + ? null + : CertInfo.fromJson(json['cert_info'] as Map), $type: json['runtimeType'] as String?, ); Map _$$_ExamineResultToJson(_$_ExamineResult instance) => { 'password': instance.password, - 'private_key': instance.privateKey, - 'certificates': instance.certificates, + 'key_type': _$KeyTypeEnumMap[instance.keyType], + 'cert_info': instance.certInfo, 'runtimeType': instance.$type, }; diff --git a/lib/piv/views/actions.dart b/lib/piv/views/actions.dart index 19bebbc2..183e711c 100644 --- a/lib/piv/views/actions.dart +++ b/lib/piv/views/actions.dart @@ -130,7 +130,8 @@ Widget registerPivActions( }); }), ImportIntent: CallbackAction(onInvoke: (intent) async { - if (!await _authIfNeeded(ref, devicePath, pivState)) { + if (!pivState.protectedKey && + !await _authIfNeeded(ref, devicePath, pivState)) { return false; } @@ -198,9 +199,11 @@ Widget registerPivActions( return true; }), DeleteIntent: CallbackAction(onInvoke: (_) async { - if (!await _authIfNeeded(ref, devicePath, pivState)) { + if (!pivState.protectedKey && + !await _authIfNeeded(ref, devicePath, pivState)) { return false; } + final withContext = ref.read(withContextProvider); final bool? deleted = await withContext((context) async => await showBlurDialog( diff --git a/lib/piv/views/cert_info_view.dart b/lib/piv/views/cert_info_view.dart new file mode 100644 index 00000000..013dc742 --- /dev/null +++ b/lib/piv/views/cert_info_view.dart @@ -0,0 +1,99 @@ +/* + * Copyright (C) 2023 Yubico. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import 'package:flutter/material.dart'; +import 'package:flutter_gen/gen_l10n/app_localizations.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:intl/intl.dart'; +import 'package:yubico_authenticator/app/message.dart'; +import 'package:yubico_authenticator/app/state.dart'; +import 'package:yubico_authenticator/piv/models.dart'; +import 'package:yubico_authenticator/widgets/tooltip_if_truncated.dart'; + +class CertInfoTable extends ConsumerWidget { + final CertInfo certInfo; + + const CertInfoTable(this.certInfo, {super.key}); + + @override + Widget build(BuildContext context, WidgetRef ref) { + final l10n = AppLocalizations.of(context)!; + final textTheme = Theme.of(context).textTheme; + // This is what ListTile uses for subtitle + final subtitleStyle = textTheme.bodyMedium!.copyWith( + color: textTheme.bodySmall!.color, + ); + final dateFormat = DateFormat.yMMMEd(); + final clipboard = ref.watch(clipboardProvider); + final withContext = ref.watch(withContextProvider); + + Widget header(String title) => Text( + title, + textAlign: TextAlign.right, + ); + + Widget body(String title, String value) => GestureDetector( + onDoubleTap: () async { + await clipboard.setText(value); + if (!clipboard.platformGivesFeedback()) { + await withContext((context) async { + showMessage(context, l10n.p_target_copied_clipboard(title)); + }); + } + }, + child: TooltipIfTruncated( + text: value, + style: subtitleStyle, + tooltip: value.replaceAllMapped( + RegExp(r',([A-Z]+)='), (match) => '\n${match[1]}='), + ), + ); + + return Row( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Column( + crossAxisAlignment: CrossAxisAlignment.end, + children: [ + header(l10n.s_subject), + header(l10n.s_issuer), + header(l10n.s_serial), + header(l10n.s_certificate_fingerprint), + header(l10n.s_valid_from), + header(l10n.s_valid_to), + ], + ), + const SizedBox(width: 8), + Flexible( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + body(l10n.s_subject, certInfo.subject), + body(l10n.s_issuer, certInfo.issuer), + body(l10n.s_serial, certInfo.serial), + body(l10n.s_certificate_fingerprint, certInfo.fingerprint), + body(l10n.s_valid_from, + dateFormat.format(DateTime.parse(certInfo.notValidBefore))), + body(l10n.s_valid_to, + dateFormat.format(DateTime.parse(certInfo.notValidAfter))), + ], + ), + ), + ], + ); + } +} diff --git a/lib/piv/views/import_file_dialog.dart b/lib/piv/views/import_file_dialog.dart index 1af99ef9..92e616e1 100644 --- a/lib/piv/views/import_file_dialog.dart +++ b/lib/piv/views/import_file_dialog.dart @@ -25,6 +25,7 @@ import '../../widgets/responsive_dialog.dart'; import '../models.dart'; import '../state.dart'; import '../keys.dart' as keys; +import 'cert_info_view.dart'; class ImportFileDialog extends ConsumerStatefulWidget { final DevicePath devicePath; @@ -77,6 +78,11 @@ class _ImportFileDialogState extends ConsumerState { @override Widget build(BuildContext context) { final l10n = AppLocalizations.of(context)!; + final textTheme = Theme.of(context).textTheme; + // This is what ListTile uses for subtitle + final subtitleStyle = textTheme.bodyMedium!.copyWith( + color: textTheme.bodySmall!.color, + ); final state = _state; if (state == null) { return ResponsiveDialog( @@ -141,7 +147,7 @@ class _ImportFileDialogState extends ConsumerState { ), ), ), - result: (_, privateKey, certificates) => ResponsiveDialog( + result: (_, keyType, certInfo) => ResponsiveDialog( title: Text(l10n.l_import_file), actions: [ TextButton( @@ -171,8 +177,37 @@ class _ImportFileDialogState extends ConsumerState { children: [ Text(l10n.p_import_items_desc( widget.pivSlot.slot.getDisplayName(l10n))), - if (privateKey) Text(l10n.l_bullet(l10n.s_private_key)), - if (certificates > 0) Text(l10n.l_bullet(l10n.s_certificate)), + if (keyType != null) ...[ + Text( + l10n.s_private_key, + style: textTheme.bodyLarge, + softWrap: true, + textAlign: TextAlign.center, + ), + Row( + mainAxisSize: MainAxisSize.min, + children: [ + Text(l10n.s_algorithm), + const SizedBox(width: 8), + Text( + keyType.name.toUpperCase(), + style: subtitleStyle, + ), + ], + ) + ], + if (certInfo != null) ...[ + Text( + l10n.s_certificate, + style: textTheme.bodyLarge, + softWrap: true, + textAlign: TextAlign.center, + ), + SizedBox( + height: 120, // Needed for layout, adapt if text sizes changes + child: CertInfoTable(certInfo), + ), + ] ] .map((e) => Padding( padding: const EdgeInsets.symmetric(vertical: 8.0), diff --git a/lib/piv/views/slot_dialog.dart b/lib/piv/views/slot_dialog.dart index 7477f847..31f0f460 100644 --- a/lib/piv/views/slot_dialog.dart +++ b/lib/piv/views/slot_dialog.dart @@ -17,16 +17,14 @@ import 'package:flutter/material.dart'; import 'package:flutter_gen/gen_l10n/app_localizations.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:intl/intl.dart'; -import '../../app/message.dart'; import '../../app/state.dart'; import '../../app/views/fs_dialog.dart'; import '../../app/views/action_list.dart'; -import '../../widgets/tooltip_if_truncated.dart'; import '../models.dart'; import '../state.dart'; import 'actions.dart'; +import 'cert_info_view.dart'; class SlotDialog extends ConsumerWidget { final SlotId pivSlot; @@ -48,8 +46,6 @@ class SlotDialog extends ConsumerWidget { final subtitleStyle = textTheme.bodyMedium!.copyWith( color: textTheme.bodySmall!.color, ); - final clipboard = ref.watch(clipboardProvider); - final withContext = ref.read(withContextProvider); final pivState = ref.watch(pivStateProvider(node.path)).valueOrNull; final slotData = ref.watch(pivSlotsProvider(node.path).select((value) => @@ -61,34 +57,6 @@ class SlotDialog extends ConsumerWidget { return const FsDialog(child: CircularProgressIndicator()); } - TableRow detailRow(String title, String value) { - return TableRow( - children: [ - Text( - l10n.s_definition(title), - textAlign: TextAlign.right, - ), - const SizedBox(width: 8.0), - GestureDetector( - onDoubleTap: () async { - await clipboard.setText(value); - if (!clipboard.platformGivesFeedback()) { - await withContext((context) async { - showMessage(context, l10n.p_target_copied_clipboard(title)); - }); - } - }, - child: TooltipIfTruncated( - text: value, - style: subtitleStyle, - tooltip: value.replaceAllMapped( - RegExp(r',([A-Z]+)='), (match) => '\n${match[1]}='), - ), - ), - ], - ); - } - final certInfo = slotData.certInfo; return registerPivActions( node.path, @@ -113,27 +81,7 @@ class SlotDialog extends ConsumerWidget { if (certInfo != null) ...[ Padding( padding: const EdgeInsets.all(16), - child: Table( - defaultColumnWidth: const IntrinsicColumnWidth(), - columnWidths: const {2: FlexColumnWidth()}, - children: [ - detailRow(l10n.s_subject, certInfo.subject), - detailRow(l10n.s_issuer, certInfo.issuer), - detailRow(l10n.s_serial, certInfo.serial), - detailRow(l10n.s_certificate_fingerprint, - certInfo.fingerprint), - detailRow( - l10n.s_valid_from, - DateFormat.yMMMEd().format( - DateTime.parse(certInfo.notValidBefore)), - ), - detailRow( - l10n.s_valid_to, - DateFormat.yMMMEd().format( - DateTime.parse(certInfo.notValidAfter)), - ), - ], - ), + child: CertInfoTable(certInfo), ), ] else ...[ Padding(