From b7f8853ef28a3a8880bc9dbe93683b2be90d6eb9 Mon Sep 17 00:00:00 2001 From: Geoffroy BONNEVILLE Date: Mon, 18 May 2020 22:20:31 +0200 Subject: [PATCH] WIP Protect/Unprotect Additional Field on selection --- ModernKeePass.Application/Application.csproj | 1 + .../Common/Interfaces/IFileProxy.cs | 1 + .../Common/Interfaces/ILogger.cs | 12 +++ .../SaveDatabase/SaveDatabaseCommand.cs | 14 ++- .../KeePass/KeePassDatabaseClient.cs | 6 +- .../UWP/StorageFileClient.cs | 18 ++++ .../UWP/UwpCryptographyClient.cs | 32 +++---- ModernKeePass/App.xaml.cs | 12 +-- ModernKeePass/DependencyInjection.cs | 3 + ModernKeePass/Log/HockeyAppLog.cs | 43 +++++++++ ModernKeePass/ViewModels/EntryDetailVm.cs | 91 +++++++++++++------ ModernKeePass/Views/EntryDetailPage.xaml | 7 +- .../UserControls/ColorPickerUserControl.xaml | 4 +- .../ColorPickerUserControl.xaml.cs | 11 --- ModernKeePass/Win81App.csproj | 1 + WinAppCommon/ViewModels/Items/EntryFieldVm.cs | 9 +- .../ViewModels/UserControls/ColorPickerVm.cs | 37 ++++++++ 17 files changed, 227 insertions(+), 75 deletions(-) create mode 100644 ModernKeePass.Application/Common/Interfaces/ILogger.cs create mode 100644 ModernKeePass/Log/HockeyAppLog.cs create mode 100644 WinAppCommon/ViewModels/UserControls/ColorPickerVm.cs diff --git a/ModernKeePass.Application/Application.csproj b/ModernKeePass.Application/Application.csproj index 1d6c67b..b3864be 100644 --- a/ModernKeePass.Application/Application.csproj +++ b/ModernKeePass.Application/Application.csproj @@ -83,6 +83,7 @@ + diff --git a/ModernKeePass.Application/Common/Interfaces/IFileProxy.cs b/ModernKeePass.Application/Common/Interfaces/IFileProxy.cs index 7fe7820..1014501 100644 --- a/ModernKeePass.Application/Common/Interfaces/IFileProxy.cs +++ b/ModernKeePass.Application/Common/Interfaces/IFileProxy.cs @@ -10,6 +10,7 @@ namespace ModernKeePass.Application.Common.Interfaces Task CreateFile(string name, string extension, string description, bool addToRecent); Task ReadBinaryFile(string path); Task> ReadTextFile(string path); + Task WriteToLogFile(IEnumerable data); Task WriteBinaryContentsToFile(string path, byte[] contents); void ReleaseFile(string path); } diff --git a/ModernKeePass.Application/Common/Interfaces/ILogger.cs b/ModernKeePass.Application/Common/Interfaces/ILogger.cs new file mode 100644 index 0000000..cc211fc --- /dev/null +++ b/ModernKeePass.Application/Common/Interfaces/ILogger.cs @@ -0,0 +1,12 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; + +namespace ModernKeePass.Application.Common.Interfaces +{ + public interface ILogger + { + Task LogError(Exception exception); + void LogTrace(string message, Dictionary values); + } +} \ No newline at end of file diff --git a/ModernKeePass.Application/Database/Commands/SaveDatabase/SaveDatabaseCommand.cs b/ModernKeePass.Application/Database/Commands/SaveDatabase/SaveDatabaseCommand.cs index 4e5ef28..5378350 100644 --- a/ModernKeePass.Application/Database/Commands/SaveDatabase/SaveDatabaseCommand.cs +++ b/ModernKeePass.Application/Database/Commands/SaveDatabase/SaveDatabaseCommand.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.Diagnostics; using MediatR; using System.Threading.Tasks; using ModernKeePass.Application.Common.Interfaces; @@ -14,11 +16,13 @@ namespace ModernKeePass.Application.Database.Commands.SaveDatabase { private readonly IDatabaseProxy _database; private readonly IFileProxy _file; + private readonly ILogger _logger; - public SaveDatabaseCommandHandler(IDatabaseProxy database, IFileProxy file) + public SaveDatabaseCommandHandler(IDatabaseProxy database, IFileProxy file, ILogger logger) { _database = database; _file = file; + _logger = logger; } public async Task Handle(SaveDatabaseCommand message) @@ -27,6 +31,7 @@ namespace ModernKeePass.Application.Database.Commands.SaveDatabase try { + var timeToSave = Stopwatch.StartNew(); if (!string.IsNullOrEmpty(message.FilePath)) { _database.FileAccessToken = message.FilePath; @@ -40,6 +45,13 @@ namespace ModernKeePass.Application.Database.Commands.SaveDatabase // Transactional write to file await _file.WriteBinaryContentsToFile(_database.FileAccessToken, contents); + timeToSave.Stop(); + + _logger.LogTrace("SaveCommand", new Dictionary + { + { "duration", timeToSave.ElapsedMilliseconds.ToString()}, + { "size", _database.Size.ToString()} + }); } catch (Exception exception) { diff --git a/ModernKeePass.Infrastructure/KeePass/KeePassDatabaseClient.cs b/ModernKeePass.Infrastructure/KeePass/KeePassDatabaseClient.cs index 43e0535..a3ffba3 100644 --- a/ModernKeePass.Infrastructure/KeePass/KeePassDatabaseClient.cs +++ b/ModernKeePass.Infrastructure/KeePass/KeePassDatabaseClient.cs @@ -253,7 +253,8 @@ namespace ModernKeePass.Infrastructure.KeePass case EntryFieldName.Password: case EntryFieldName.Notes: case EntryFieldName.Url: - var unprotectedFieldValue = isProtected ? await _cryptography.UnProtect(fieldValue.ToString()) : fieldValue.ToString(); + var stringValue = fieldValue == null ? string.Empty: fieldValue.ToString(); + var unprotectedFieldValue = isProtected ? await _cryptography.UnProtect(stringValue) : stringValue; pwEntry.Strings.Set(EntryFieldMapper.MapFieldToPwDef(fieldName), new ProtectedString(isProtected, unprotectedFieldValue)); break; case EntryFieldName.HasExpirationDate: @@ -272,7 +273,8 @@ namespace ModernKeePass.Infrastructure.KeePass pwEntry.ForegroundColor = (Color)fieldValue; break; default: - var unprotectedAdditionalFieldValue = isProtected ? await _cryptography.UnProtect(fieldValue.ToString()) : fieldValue.ToString(); + var additionalStringValue = fieldValue == null ? string.Empty: fieldValue.ToString(); + var unprotectedAdditionalFieldValue = isProtected ? await _cryptography.UnProtect(additionalStringValue) : additionalStringValue; pwEntry.Strings.Set(fieldName, new ProtectedString(isProtected, unprotectedAdditionalFieldValue)); break; } diff --git a/ModernKeePass.Infrastructure/UWP/StorageFileClient.cs b/ModernKeePass.Infrastructure/UWP/StorageFileClient.cs index bb21870..3dd1816 100644 --- a/ModernKeePass.Infrastructure/UWP/StorageFileClient.cs +++ b/ModernKeePass.Infrastructure/UWP/StorageFileClient.cs @@ -74,6 +74,24 @@ namespace ModernKeePass.Infrastructure.UWP return result; } + public async Task WriteToLogFile(IEnumerable data) + { + var local = ApplicationData.Current.LocalFolder; + var logFile = await local.CreateFileAsync("LogFile.txt", CreationCollisionOption.OpenIfExists).AsTask(); + + if (logFile != null) + { + try + { + await FileIO.AppendLinesAsync(logFile, data); + } + catch (Exception) + { + // If another option is available to the app to log error(i.e. Azure Mobile Service, etc...) then try that here + } + } + } + public void ReleaseFile(string path) { StorageApplicationPermissions.FutureAccessList.Remove(path); diff --git a/ModernKeePass.Infrastructure/UWP/UwpCryptographyClient.cs b/ModernKeePass.Infrastructure/UWP/UwpCryptographyClient.cs index 8b8e117..3d26a38 100644 --- a/ModernKeePass.Infrastructure/UWP/UwpCryptographyClient.cs +++ b/ModernKeePass.Infrastructure/UWP/UwpCryptographyClient.cs @@ -12,27 +12,21 @@ namespace ModernKeePass.Infrastructure.UWP public async Task Protect(string value) { if (string.IsNullOrEmpty(value)) return value; - try - { - // Create a DataProtectionProvider object for the specified descriptor. - var provider = new DataProtectionProvider("LOCAL=user"); - - // Encode the plaintext input message to a buffer. - var buffMsg = CryptographicBuffer.ConvertStringToBinary(value, BinaryStringEncoding.Utf8); - // Encrypt the message. - var buffProtected = await provider.ProtectAsync(buffMsg).AsTask().ConfigureAwait(false); - - // Encode buffer to Base64 - var protectedValue = CryptographicBuffer.EncodeToBase64String(buffProtected); + // Create a DataProtectionProvider object for the specified descriptor. + var provider = new DataProtectionProvider("LOCAL=user"); + + // Encode the plaintext input message to a buffer. + var buffMsg = CryptographicBuffer.ConvertStringToBinary(value, BinaryStringEncoding.Utf8); - // Return the encrypted string. - return protectedValue; - } - catch (Exception e) - { - return string.Empty; - } + // Encrypt the message. + var buffProtected = await provider.ProtectAsync(buffMsg).AsTask().ConfigureAwait(false); + + // Encode buffer to Base64 + var protectedValue = CryptographicBuffer.EncodeToBase64String(buffProtected); + + // Return the encrypted string. + return protectedValue; } public async Task UnProtect(string value) diff --git a/ModernKeePass/App.xaml.cs b/ModernKeePass/App.xaml.cs index 174ee88..64df110 100644 --- a/ModernKeePass/App.xaml.cs +++ b/ModernKeePass/App.xaml.cs @@ -38,11 +38,12 @@ namespace ModernKeePass private readonly IMediator _mediator; private readonly ISettingsProxy _settings; private readonly INavigationService _navigation; - private readonly IHockeyClient _hockey; private readonly INotificationService _notification; private readonly IFileProxy _file; private readonly IMessenger _messenger; - + private readonly ILogger _log; + private readonly IHockeyClient _hockey; + /// /// Initializes the singleton application object. This is the first line of authored code /// executed, and as such is the logical equivalent of main() or WinMain(). @@ -63,6 +64,7 @@ namespace ModernKeePass _settings = Services.GetService(); _navigation = Services.GetService(); _notification = Services.GetService(); + _log = Services.GetService(); _hockey = Services.GetService(); _file = Services.GetService(); _messenger = Services.GetService(); @@ -93,8 +95,7 @@ namespace ModernKeePass private void OnUnhandledException(object sender, UnhandledExceptionEventArgs e) { - _hockey.TrackException(e.Exception); - _hockey.Flush(); + _log.LogError(e.Exception); } /// @@ -209,8 +210,7 @@ namespace ModernKeePass } catch (Exception ex) { - _hockey.TrackException(ex); - _hockey.Flush(); + _log.LogError(ex); } finally { diff --git a/ModernKeePass/DependencyInjection.cs b/ModernKeePass/DependencyInjection.cs index 7420f02..bfaed91 100644 --- a/ModernKeePass/DependencyInjection.cs +++ b/ModernKeePass/DependencyInjection.cs @@ -4,8 +4,10 @@ using GalaSoft.MvvmLight.Messaging; using GalaSoft.MvvmLight.Views; using Microsoft.Extensions.DependencyInjection; using Microsoft.HockeyApp; +using ModernKeePass.Application.Common.Interfaces; using ModernKeePass.Common; using ModernKeePass.Views; +using ModernKeePass.Log; namespace ModernKeePass { @@ -37,6 +39,7 @@ namespace ModernKeePass #endif return HockeyClient.Current; }); + services.AddSingleton(typeof(ILogger), typeof(HockeyAppLog)); return services; } diff --git a/ModernKeePass/Log/HockeyAppLog.cs b/ModernKeePass/Log/HockeyAppLog.cs new file mode 100644 index 0000000..783ceb6 --- /dev/null +++ b/ModernKeePass/Log/HockeyAppLog.cs @@ -0,0 +1,43 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using ModernKeePass.Application.Common.Interfaces; +using Microsoft.HockeyApp; +using ModernKeePass.Domain.Interfaces; + +namespace ModernKeePass.Log +{ + public class HockeyAppLog : ILogger + { + private readonly IHockeyClient _hockey; + private readonly IFileProxy _file; + private readonly IDateTime _dateTime; + + public HockeyAppLog(IHockeyClient hockey, IFileProxy file, IDateTime dateTime) + { + _hockey = hockey; + _file = file; + _dateTime = dateTime; + } + + public async Task LogError(Exception exception) + { + _hockey.TrackException(exception); + _hockey.Flush(); + + var time = _dateTime.Now.ToLocalTime(); + var data = new List + { + $"{time} - {exception.Message}", + $"{time} - {exception.StackTrace}" + }; + await _file.WriteToLogFile(data); + } + + public void LogTrace(string message, Dictionary values) + { + _hockey.TrackTrace(message, values); + _hockey.Flush(); + } + } +} diff --git a/ModernKeePass/ViewModels/EntryDetailVm.cs b/ModernKeePass/ViewModels/EntryDetailVm.cs index 06b33fc..f3e961f 100644 --- a/ModernKeePass/ViewModels/EntryDetailVm.cs +++ b/ModernKeePass/ViewModels/EntryDetailVm.cs @@ -35,7 +35,6 @@ using ModernKeePass.Domain.Dtos; using ModernKeePass.Domain.Exceptions; using ModernKeePass.Extensions; using ModernKeePass.Models; -using ModernKeePass.ViewModels.ListItems; namespace ModernKeePass.ViewModels { @@ -108,7 +107,7 @@ namespace ModernKeePass.ViewModels } public ObservableCollection History { get; private set; } - public ObservableCollection AdditionalFields { get; private set; } + public ObservableCollection AdditionalFields { get; private set; } public ObservableCollection Attachments { get; private set; } /// @@ -124,14 +123,8 @@ namespace ModernKeePass.ViewModels Set(() => SelectedItem, ref _selectedItem, value, true); if (value != null) { - AdditionalFields = - new ObservableCollection( - SelectedItem.AdditionalFields.Select(f => - { - var field = new EntryFieldVm(_cryptography); - field.Initialize(f.Name, f.Value, f.IsProtected); - return field; - })); + AdditionalFields = new ObservableCollection(SelectedItem.AdditionalFields); + Attachments = new ObservableCollection(SelectedItem.Attachments.Select(f => new Attachment { Name = f.Key, @@ -164,6 +157,13 @@ namespace ModernKeePass.ViewModels { Set(() => AdditionalFieldSelectedIndex, ref _additionalFieldSelectedIndex, value); DeleteAdditionalField.RaiseCanExecuteChanged(); + if (value != -1) + { + var additionalField = AdditionalFields[value]; + Set(nameof(AdditionalFieldName), ref _additionalFieldName, additionalField.Name); + Set(nameof(AdditionalFieldValue), ref _additionalFieldValue, additionalField.IsProtected ? _cryptography.UnProtect(additionalField.Value).GetAwaiter().GetResult() : additionalField.Value); + Set(nameof(AdditionalFieldIsProtected), ref _additionalFieldIsProtected, additionalField.IsProtected); + } } } @@ -173,7 +173,7 @@ namespace ModernKeePass.ViewModels set { SelectedItem.Title.Value = value; - SetFieldValue(nameof(Title), value, false).Wait(); + SetFieldValue(nameof(Title), value, false).ConfigureAwait(false).GetAwaiter(); } } @@ -183,7 +183,7 @@ namespace ModernKeePass.ViewModels set { SelectedItem.Username.Value = value; - SetFieldValue(nameof(UserName), value, false).Wait(); + SetFieldValue(nameof(UserName), value, false).ConfigureAwait(false).GetAwaiter(); RaisePropertyChanged(nameof(UserName)); } } @@ -193,9 +193,10 @@ namespace ModernKeePass.ViewModels get { return _cryptography.UnProtect(SelectedItem.Password.Value).GetAwaiter().GetResult(); } set { - var protectedPassword = _cryptography.Protect(value).GetAwaiter().GetResult(); + // TODO: cleanup this + var protectedPassword = _cryptography.Protect(value).ConfigureAwait(false).GetAwaiter().GetResult(); SelectedItem.Password.Value = protectedPassword; - SetFieldValue(nameof(Password), protectedPassword, true).Wait(); + SetFieldValue(nameof(Password), value, true).ConfigureAwait(false).GetAwaiter(); RaisePropertyChanged(nameof(Password)); RaisePropertyChanged(nameof(PasswordComplexityIndicator)); } @@ -207,7 +208,7 @@ namespace ModernKeePass.ViewModels set { SelectedItem.Url.Value = value; - SetFieldValue(nameof(Url), value, false).Wait(); + SetFieldValue(nameof(Url), value, false).ConfigureAwait(false).GetAwaiter(); RaisePropertyChanged(nameof(Url)); } } @@ -218,7 +219,7 @@ namespace ModernKeePass.ViewModels set { SelectedItem.Notes.Value = value; - SetFieldValue(nameof(Notes), value, false).Wait(); + SetFieldValue(nameof(Notes), value, false).ConfigureAwait(false).GetAwaiter(); } } @@ -228,7 +229,7 @@ namespace ModernKeePass.ViewModels set { SelectedItem.Icon = (Icon)Enum.Parse(typeof(Icon), value.ToString()); - SetFieldValue(nameof(Icon), SelectedItem.Icon, false).Wait(); + SetFieldValue(nameof(Icon), SelectedItem.Icon, false).ConfigureAwait(false).GetAwaiter(); } } @@ -240,7 +241,7 @@ namespace ModernKeePass.ViewModels if (!HasExpirationDate) return; SelectedItem.ExpirationDate = value.Date; - SetFieldValue("ExpirationDate", SelectedItem.ExpirationDate, false).Wait(); + SetFieldValue("ExpirationDate", SelectedItem.ExpirationDate, false).ConfigureAwait(false).GetAwaiter(); } } @@ -252,7 +253,7 @@ namespace ModernKeePass.ViewModels if (!HasExpirationDate) return; SelectedItem.ExpirationDate = SelectedItem.ExpirationDate.Date.Add(value); - SetFieldValue("ExpirationDate", SelectedItem.ExpirationDate, false).Wait(); + SetFieldValue("ExpirationDate", SelectedItem.ExpirationDate, false).ConfigureAwait(false).GetAwaiter(); } } @@ -262,7 +263,7 @@ namespace ModernKeePass.ViewModels set { SelectedItem.HasExpirationDate = value; - SetFieldValue(nameof(HasExpirationDate), value, false).Wait(); + SetFieldValue(nameof(HasExpirationDate), value, false).ConfigureAwait(false).GetAwaiter(); RaisePropertyChanged(nameof(HasExpirationDate)); } } @@ -273,7 +274,7 @@ namespace ModernKeePass.ViewModels set { SelectedItem.BackgroundColor = value.ToColor(); - SetFieldValue(nameof(BackgroundColor), SelectedItem.BackgroundColor, false).Wait(); + SetFieldValue(nameof(BackgroundColor), SelectedItem.BackgroundColor, false).ConfigureAwait(false).GetAwaiter(); } } @@ -283,7 +284,7 @@ namespace ModernKeePass.ViewModels set { SelectedItem.ForegroundColor = value.ToColor(); - SetFieldValue(nameof(ForegroundColor), SelectedItem.ForegroundColor, false).Wait(); + SetFieldValue(nameof(ForegroundColor), SelectedItem.ForegroundColor, false).ConfigureAwait(false).GetAwaiter(); } } @@ -299,6 +300,41 @@ namespace ModernKeePass.ViewModels set { Set(() => IsRevealPassword, ref _isRevealPassword, value); } } + private string _additionalFieldName; + private string _additionalFieldValue; + private bool _additionalFieldIsProtected; + + public string AdditionalFieldName + { + get { return _additionalFieldName; } + set + { + var newName = EntryFieldName.StandardFieldNames.Contains(value) ? $"{value}_1" : value; + UpdateFieldName(_additionalFieldName, newName, AdditionalFieldValue, AdditionalFieldIsProtected).ConfigureAwait(false).GetAwaiter(); + } + } + + public string AdditionalFieldValue + { + get + { + return _additionalFieldValue; + } + set + { + SetFieldValue(AdditionalFieldName, value, AdditionalFieldIsProtected).ConfigureAwait(false).GetAwaiter(); + } + } + + public bool AdditionalFieldIsProtected + { + get { return _additionalFieldIsProtected; } + set + { + SetFieldValue(AdditionalFieldName, AdditionalFieldValue, value).ConfigureAwait(false).GetAwaiter(); + } + } + public RelayCommand SaveCommand { get; } public RelayCommand GeneratePasswordCommand { get; } public RelayCommand MoveCommand { get; } @@ -307,7 +343,7 @@ namespace ModernKeePass.ViewModels public RelayCommand GoBackCommand { get; } public RelayCommand GoToParentCommand { get; set; } public RelayCommand AddAdditionalField { get; set; } - public RelayCommand DeleteAdditionalField { get; set; } + public RelayCommand DeleteAdditionalField { get; set; } public RelayCommand OpenAttachmentCommand { get; set; } public RelayCommand AddAttachmentCommand { get; set; } public RelayCommand DeleteAttachmentCommand { get; set; } @@ -349,7 +385,7 @@ namespace ModernKeePass.ViewModels GoBackCommand = new RelayCommand(() => _navigation.GoBack()); GoToParentCommand = new RelayCommand(() => GoToGroup(_parent.Id)); AddAdditionalField = new RelayCommand(AddField, () => IsCurrentEntry); - DeleteAdditionalField = new RelayCommand(async field => await DeleteField(field), field => field != null && IsCurrentEntry); + DeleteAdditionalField = new RelayCommand(async field => await DeleteField(field), field => field != null && IsCurrentEntry); OpenAttachmentCommand = new RelayCommand(async attachment => await OpenAttachment(attachment)); AddAttachmentCommand = new RelayCommand(async () => await AddAttachment(), () => IsCurrentEntry); DeleteAttachmentCommand = new RelayCommand(async attachment => await DeleteAttachment(attachment), _ => IsCurrentEntry); @@ -412,7 +448,8 @@ namespace ModernKeePass.ViewModels private async Task SetFieldValue(string fieldName, object value, bool isProtected) { - await _mediator.Send(new UpsertFieldCommand { EntryId = Id, FieldName = fieldName, FieldValue = value, IsProtected = isProtected}); + var protectedValue = isProtected ? await _cryptography.Protect(value?.ToString()) : value; + await _mediator.Send(new UpsertFieldCommand { EntryId = Id, FieldName = fieldName, FieldValue = protectedValue, IsProtected = isProtected}); UpdateDirtyStatus(true); } @@ -424,11 +461,11 @@ namespace ModernKeePass.ViewModels private void AddField() { - AdditionalFields.Add(new EntryFieldVm(_cryptography)); + AdditionalFields.Add(new FieldVm()); AdditionalFieldSelectedIndex = AdditionalFields.Count - 1; } - private async Task DeleteField(EntryFieldVm field) + private async Task DeleteField(FieldVm field) { AdditionalFields.Remove(field); if (!string.IsNullOrEmpty(field.Name)) diff --git a/ModernKeePass/Views/EntryDetailPage.xaml b/ModernKeePass/Views/EntryDetailPage.xaml index 11b41fd..fde95d6 100644 --- a/ModernKeePass/Views/EntryDetailPage.xaml +++ b/ModernKeePass/Views/EntryDetailPage.xaml @@ -321,7 +321,6 @@ - @@ -506,12 +505,12 @@ - - +