From ca61fa9d1dea16d097cfad786d050ce4eb659379 Mon Sep 17 00:00:00 2001 From: Philipp Crocoll Date: Sat, 2 Jan 2021 12:31:19 +0100 Subject: [PATCH] several changes to fix https://github.com/PhilippC/keepass2android/issues/1399: * adding a lock to avoid flickering/disappearing prompt * returning a fill-response instead of a dataset from authentication activity * immediately present one search result (if there is a match) as a dataset item in the autofill prompt --- .../services/AutofillBase/AutofillHelper.cs | 1 + .../AutofillBase/AutofillServiceBase.cs | 216 ++++++++++-------- .../ChooseForAutofillActivityBase.cs | 5 +- .../model/FilledAutofillFieldCollection.cs | 4 +- .../Kp2aAutofill/Kp2aAutofillService.cs | 22 +- 5 files changed, 147 insertions(+), 101 deletions(-) diff --git a/src/keepass2android/services/AutofillBase/AutofillHelper.cs b/src/keepass2android/services/AutofillBase/AutofillHelper.cs index 003677ff..6b6233c0 100644 --- a/src/keepass2android/services/AutofillBase/AutofillHelper.cs +++ b/src/keepass2android/services/AutofillBase/AutofillHelper.cs @@ -28,6 +28,7 @@ namespace keepass2android.services.AutofillBase if (datasetName != null) { var datasetBuilder = new Dataset.Builder(NewRemoteViews(context.PackageName, datasetName, intentBuilder.AppIconResource)); + datasetBuilder.SetId(datasetName); var setValueAtLeastOnce = filledAutofillFieldCollection.ApplyToFields(autofillFields, datasetBuilder); if (setValueAtLeastOnce) diff --git a/src/keepass2android/services/AutofillBase/AutofillServiceBase.cs b/src/keepass2android/services/AutofillBase/AutofillServiceBase.cs index 1dda716d..7ef6a32d 100644 --- a/src/keepass2android/services/AutofillBase/AutofillServiceBase.cs +++ b/src/keepass2android/services/AutofillBase/AutofillServiceBase.cs @@ -10,6 +10,7 @@ using Android.Service.Autofill; using Android.Util; using Android.Views.Autofill; using Android.Widget; +using Java.Util.Concurrent.Atomic; using keepass2android.services.AutofillBase.model; namespace keepass2android.services.AutofillBase @@ -30,6 +31,10 @@ namespace keepass2android.services.AutofillBase public abstract class AutofillServiceBase: AutofillService { + //use a lock to avoid returning a response several times in buggy Firefox during one connection: this avoids flickering + //and disappearing of the autofill prompt. + private AtomicBoolean _lock = new AtomicBoolean(); + public AutofillServiceBase() { @@ -83,127 +88,150 @@ namespace keepass2android.services.AutofillBase { bool isManual = (request.Flags & FillRequest.FlagManualRequest) != 0; CommonUtil.logd( "onFillRequest " + (isManual ? "manual" : "auto")); - var structure = request.FillContexts[request.FillContexts.Count - 1].Structure; + var structure = request.FillContexts.Last().Structure; - //TODO support package signature verification as soon as this is supported in Keepass storage - - var clientState = request.ClientState; - CommonUtil.logd( "onFillRequest(): data=" + CommonUtil.BundleToString(clientState)); - - - cancellationSignal.CancelEvent += (sender, e) => { - Log.Warn(CommonUtil.Tag, "Cancel autofill not implemented yet."); - }; - // Parse AutoFill data in Activity - StructureParser.AutofillTargetId query = null; - var parser = new StructureParser(this, structure); - try + if (!_lock.Get()) { - query = parser.ParseForFill(isManual); - - } - catch (Java.Lang.SecurityException e) - { - Log.Warn(CommonUtil.Tag, "Security exception handling request"); - callback.OnFailure(e.Message); - return; - } - - AutofillFieldMetadataCollection autofillFields = parser.AutofillFields; - - - var autofillIds = autofillFields.GetAutofillIds(); - if (autofillIds.Length != 0 && CanAutofill(query, isManual)) - { - var responseBuilder = new FillResponse.Builder(); + _lock.Set(true); - Dataset entryDataset = null; - if (query.IncompatiblePackageAndDomain == false) + //TODO support package signature verification as soon as this is supported in Keepass storage + + var clientState = request.ClientState; + CommonUtil.logd("onFillRequest(): data=" + CommonUtil.BundleToString(clientState)); + + + cancellationSignal.CancelEvent += (sender, e) => { - //domain and package are compatible. Use Domain if available and package otherwise. Can fill without warning. - entryDataset = BuildEntryDataset(query.DomainOrPackage, query.WebDomain, query.PackageName, autofillIds, parser, DisplayWarning.None); + Log.Warn(CommonUtil.Tag, "Cancel autofill not implemented yet."); + }; + // Parse AutoFill data in Activity + StructureParser.AutofillTargetId query = null; + var parser = new StructureParser(this, structure); + try + { + query = parser.ParseForFill(isManual); + } - else + catch (Java.Lang.SecurityException e) { - - //domain or package are incompatible. Don't show the entry. (Tried to do so first but behavior was not consistent) - //entryDataset = BuildEntryDataset(query.WebDomain, query.WebDomain, query.PackageName, autofillIds, parser, DisplayWarning.FillDomainInUntrustedApp); + Log.Warn(CommonUtil.Tag, "Security exception handling request"); + callback.OnFailure(e.Message); + return; } - bool hasEntryDataset = entryDataset != null; - if (entryDataset != null) - responseBuilder.AddDataset(entryDataset); - if (query.WebDomain != null) - AddQueryDataset(query.WebDomain, - query.WebDomain, query.PackageName, - isManual, autofillIds, responseBuilder, !hasEntryDataset, query.IncompatiblePackageAndDomain ? DisplayWarning.FillDomainInUntrustedApp : DisplayWarning.None); - else - AddQueryDataset(query.PackageNameWithPseudoSchema, - query.WebDomain, query.PackageName, - isManual, autofillIds, responseBuilder, !hasEntryDataset, DisplayWarning.None); + AutofillFieldMetadataCollection autofillFields = parser.AutofillFields; - AddDisableDataset(query.DomainOrPackage, autofillIds, responseBuilder, isManual); - - if (PreferenceManager.GetDefaultSharedPreferences(this) - .GetBoolean(GetString(Resource.String.OfferSaveCredentials_key), true)) + var autofillIds = autofillFields.GetAutofillIds(); + if (autofillIds.Length != 0 && CanAutofill(query, isManual)) { - if (!CompatBrowsers.Contains(parser.PackageId)) + var responseBuilder = new FillResponse.Builder(); + + bool hasEntryDataset = false; + + if (query.IncompatiblePackageAndDomain == false) { - responseBuilder.SetSaveInfo(new SaveInfo.Builder(parser.AutofillFields.SaveType, - parser.AutofillFields.GetAutofillIds()).Build()); + //domain and package are compatible. Use Domain if available and package otherwise. Can fill without warning. + foreach (var entryDataset in BuildEntryDatasets(query.DomainOrPackage, query.WebDomain, + query.PackageName, + autofillIds, parser, DisplayWarning.None) + ) + { + responseBuilder.AddDataset(entryDataset); + hasEntryDataset = true; + } } - - } + - callback.OnSuccess(responseBuilder.Build()); - } - else - { - callback.OnSuccess(null); + + { + if (query.WebDomain != null) + AddQueryDataset(query.WebDomain, + query.WebDomain, query.PackageName, + isManual, autofillIds, responseBuilder, !hasEntryDataset, + query.IncompatiblePackageAndDomain + ? DisplayWarning.FillDomainInUntrustedApp + : DisplayWarning.None); + else + AddQueryDataset(query.PackageNameWithPseudoSchema, + query.WebDomain, query.PackageName, + isManual, autofillIds, responseBuilder, !hasEntryDataset, DisplayWarning.None); + } + + AddDisableDataset(query.DomainOrPackage, autofillIds, responseBuilder, isManual); + + if (PreferenceManager.GetDefaultSharedPreferences(this) + .GetBoolean(GetString(Resource.String.OfferSaveCredentials_key), true)) + { + if (!CompatBrowsers.Contains(parser.PackageId)) + { + responseBuilder.SetSaveInfo(new SaveInfo.Builder(parser.AutofillFields.SaveType, + parser.AutofillFields.GetAutofillIds()).Build()); + } + + } + + callback.OnSuccess(responseBuilder.Build()); + } + else + { + callback.OnSuccess(null); + } } } - private Dataset BuildEntryDataset(string query, string queryDomain, string queryPackage, AutofillId[] autofillIds, StructureParser parser, + private List BuildEntryDatasets(string query, string queryDomain, string queryPackage, AutofillId[] autofillIds, StructureParser parser, DisplayWarning warning) { - var filledAutofillFieldCollection = GetSuggestedEntry(query); - if (filledAutofillFieldCollection == null) - return null; - - if (warning == DisplayWarning.None) + List result = new List(); + var suggestedEntries = GetSuggestedEntries(query).ToDictionary(e => e.DatasetName, e => e); + foreach (var filledAutofillFieldCollection in suggestedEntries.Values) { - //can return an actual dataset - int partitionIndex = AutofillHintsHelper.GetPartitionIndex(parser.AutofillFields.FocusedAutofillCanonicalHints.FirstOrDefault()); - FilledAutofillFieldCollection partitionData = AutofillHintsHelper.FilterForPartition(filledAutofillFieldCollection, partitionIndex); - return AutofillHelper.NewDataset(this, parser.AutofillFields, partitionData, IntentBuilder); - } - else - { - //return an "auth" dataset (actually for just warning the user in case domain/package dont match) - var sender = IntentBuilder.GetAuthIntentSenderForWarning(this, query, queryDomain, queryPackage, warning); - var datasetName = filledAutofillFieldCollection.DatasetName; - if (datasetName == null) - return null; + if (filledAutofillFieldCollection == null) + continue; - RemoteViews presentation = AutofillHelper.NewRemoteViews(PackageName, datasetName, AppNames.LauncherIcon); - - var datasetBuilder = new Dataset.Builder(presentation); - datasetBuilder.SetAuthentication(sender); - //need to add placeholders so we can directly fill after ChooseActivity - foreach (var autofillId in autofillIds) + if (warning == DisplayWarning.None) { - datasetBuilder.SetValue(autofillId, AutofillValue.ForText("PLACEHOLDER")); - } + //can return an actual dataset + int partitionIndex = + AutofillHintsHelper.GetPartitionIndex(parser.AutofillFields.FocusedAutofillCanonicalHints + .FirstOrDefault()); + FilledAutofillFieldCollection partitionData = + AutofillHintsHelper.FilterForPartition(filledAutofillFieldCollection, partitionIndex); - return datasetBuilder.Build(); + result.Add(AutofillHelper.NewDataset(this, parser.AutofillFields, partitionData, IntentBuilder)); + } + else + { + //return an "auth" dataset (actually for just warning the user in case domain/package dont match) + var sender = + IntentBuilder.GetAuthIntentSenderForWarning(this, query, queryDomain, queryPackage, warning); + var datasetName = filledAutofillFieldCollection.DatasetName; + if (datasetName == null) + return null; + + RemoteViews presentation = + AutofillHelper.NewRemoteViews(PackageName, datasetName, AppNames.LauncherIcon); + + var datasetBuilder = new Dataset.Builder(presentation); + datasetBuilder.SetAuthentication(sender); + //need to add placeholders so we can directly fill after ChooseActivity + foreach (var autofillId in autofillIds) + { + datasetBuilder.SetValue(autofillId, AutofillValue.ForText("PLACEHOLDER")); + } + + result.Add(datasetBuilder.Build()); + } } - + return result; + + } - protected abstract FilledAutofillFieldCollection GetSuggestedEntry(string query); + protected abstract List GetSuggestedEntries(string query); public enum DisplayWarning { @@ -335,6 +363,8 @@ namespace keepass2android.services.AutofillBase public override void OnDisconnected() { + + _lock.Set(false); CommonUtil.logd( "onDisconnected"); } diff --git a/src/keepass2android/services/AutofillBase/ChooseForAutofillActivityBase.cs b/src/keepass2android/services/AutofillBase/ChooseForAutofillActivityBase.cs index 72f9906e..0b05f6f8 100644 --- a/src/keepass2android/services/AutofillBase/ChooseForAutofillActivityBase.cs +++ b/src/keepass2android/services/AutofillBase/ChooseForAutofillActivityBase.cs @@ -182,6 +182,7 @@ namespace keepass2android.services.AutofillBase FilledAutofillFieldCollection partitionData = AutofillHintsHelper.FilterForPartition(clientFormDataMap, partitionIndex); ReplyIntent = new Intent(); SetDatasetIntent(AutofillHelper.NewDataset(this, autofillFields, partitionData, IntentBuilder)); + SetResult(Result.Ok, ReplyIntent); } protected override void OnActivityResult(int requestCode, Result resultCode, Intent data) @@ -229,7 +230,9 @@ namespace keepass2android.services.AutofillBase protected void SetDatasetIntent(Dataset dataset) { - ReplyIntent.PutExtra(AutofillManager.ExtraAuthenticationResult, dataset); + var responseBuilder = new FillResponse.Builder(); + responseBuilder.AddDataset(dataset); + ReplyIntent.PutExtra(AutofillManager.ExtraAuthenticationResult, responseBuilder.Build()); } } } \ No newline at end of file diff --git a/src/keepass2android/services/AutofillBase/model/FilledAutofillFieldCollection.cs b/src/keepass2android/services/AutofillBase/model/FilledAutofillFieldCollection.cs index 68d4d3c2..fd1f9e96 100644 --- a/src/keepass2android/services/AutofillBase/model/FilledAutofillFieldCollection.cs +++ b/src/keepass2android/services/AutofillBase/model/FilledAutofillFieldCollection.cs @@ -74,8 +74,8 @@ namespace keepass2android.services.AutofillBase.model foreach (string hint in autofillFieldMetadataCollection.AllAutofillCanonicalHints) { foreach (AutofillFieldMetadata autofillFieldMetadata in autofillFieldMetadataCollection.GetFieldsForHint(hint)) - { - FilledAutofillField filledAutofillField; + { + FilledAutofillField filledAutofillField; if (!HintMap.TryGetValue(hint, out filledAutofillField) || (filledAutofillField == null)) { continue; diff --git a/src/keepass2android/services/Kp2aAutofill/Kp2aAutofillService.cs b/src/keepass2android/services/Kp2aAutofill/Kp2aAutofillService.cs index 326edda4..05f2b3dd 100644 --- a/src/keepass2android/services/Kp2aAutofill/Kp2aAutofillService.cs +++ b/src/keepass2android/services/Kp2aAutofill/Kp2aAutofillService.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using Android; using Android.App; using Android.Content; @@ -9,6 +10,7 @@ using keepass2android.services.AutofillBase.model; using keepass2android.services.Kp2aAutofill; using Keepass2android.Pluginsdk; using KeePassLib; +using KeePassLib.Collections; using KeePassLib.Utility; using Org.Json; using AutofillServiceBase = keepass2android.services.AutofillBase.AutofillServiceBase; @@ -31,12 +33,22 @@ namespace keepass2android.services { } - protected override FilledAutofillFieldCollection GetSuggestedEntry(string query) + protected override List GetSuggestedEntries(string query) { - if (App.Kp2a.LastOpenedEntry?.SearchUrl == query) - return ChooseForAutofillActivity.GetFilledAutofillFieldCollectionFromEntry( - App.Kp2a.LastOpenedEntry, this); - return null; + var foundEntries = (ShareUrlResults.GetSearchResultsForUrl(query)?.Entries ?? new PwObjectList()) + .Select(e => new PwEntryOutput(e, App.Kp2a.FindDatabaseForElement(e))) + .ToList(); + + if ((App.Kp2a.LastOpenedEntry?.SearchUrl == query) && !foundEntries.Any(e => e.Uuid.Equals(App.Kp2a.LastOpenedEntry?.Uuid))) + { + foundEntries.Clear(); + foundEntries.Add(App.Kp2a.LastOpenedEntry); + } + + //it seems like at least with Firefox we can have at most 3 datasets. Reserve space for the disable/enable dataset and the "fill with KP2A" which allows to select another item + //so take only 1: + return foundEntries.Take(1).Select(e => ChooseForAutofillActivity.GetFilledAutofillFieldCollectionFromEntry(e, this)) + .ToList(); } protected override void HandleSaveRequest(StructureParser parser, StructureParser.AutofillTargetId query)