Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

Save new credentials from Autofill #217

Closed
7 tasks done
sashei opened this issue Nov 15, 2018 · 10 comments · Fixed by #943
Closed
7 tasks done

Save new credentials from Autofill #217

sashei opened this issue Nov 15, 2018 · 10 comments · Fixed by #943
Assignees
Labels
effort-L Expected to take a sprint for engineering to complete. feature-autofill feature-create feature-CUD priority-P1 QA-verified For the issues verified by QA

Comments

@sashei
Copy link
Contributor

sashei commented Nov 15, 2018

Acceptance Criteria

Save credentials when the user enters credentials that we don't have in Lockbox. For now, this will not respect app locked state; that will be handled in a follow-up ticket #632 or saving values from multi-screen login experiences #633

Engineering notes

Handle the onSaveRequest callback.

Todo

  • add SaveInfo to the FillResponse
  • onSuccess called in LockboxAutofillService
  • add username and pw autofill ids as parcelables to the client state in onFillRequest -- documentation
  • Modify ParsedStructure to implement findNodeById
  • Use getAutofillValues + map to ServerPassword
  • Implement DataStoreAction.Add(credential: ServerPassword)
  • Handle DataStoreAction.Add in Datastore.kt

Testing steps

The best way to test a successful save is to use the Duolingo app (it works 100% of the time)

  1. Make sure you’re signed in to lockwise and set as the autofill provider. It doesn’t matter if the app is locked or unlocked.
  2. Open the Duolingo app
  3. Sign in to your account
  4. See the popup at the bottom of the screen with the prompt “Save username and password to Lockwise?” with two buttons (“No thanks”, “Save”)
@sashei
Copy link
Contributor Author

sashei commented Nov 15, 2018

I've done some more investigation, and I think we can avoid the onSaveRequest callback simply by not passing the SaveInfo object to the FillResponse builder. I'll leave this story as a placeholder for further autofill work!!

@sashei sashei changed the title Send error when receiving onSaveRequest as AutofillService Save new credentials from Autofill Nov 15, 2018
@sandysage
Copy link

This is not a 'must have' for initial launch. It is something we need to provide prior to the Fenix launch to support our 'family of products' on Android

@sandysage sandysage added the MVP label Jan 7, 2019
@devinreams devinreams removed the MVP label Jan 15, 2019
@devinreams devinreams added this to To do in Lockwise Mobile Apr 29, 2019
@eliserichards eliserichards moved this from To do to In progress in Lockwise Mobile May 1, 2019
@devinreams devinreams removed the on-deck label May 1, 2019
@sandysage sandysage moved this from In progress to On Deck in Lockwise Mobile May 3, 2019
@devinreams devinreams moved this from On Deck to Sprint Backlog in Lockwise Mobile May 13, 2019
@eliserichards eliserichards moved this from Sprint Backlog to In Progress in Lockwise Mobile May 14, 2019
@eliserichards eliserichards self-assigned this May 15, 2019
Lockwise Mobile automation moved this from In Progress to Done May 15, 2019
@linuxwolf linuxwolf reopened this May 15, 2019
Lockwise Mobile automation moved this from Done to On Deck May 15, 2019
@eliserichards eliserichards moved this from On Deck to In Progress in Lockwise Mobile May 15, 2019
@eliserichards eliserichards moved this from Sprint Backlog to On Deck in Lockwise Mobile Sep 10, 2019
@changecourse
Copy link
Contributor

@eliserichards looking back through this (for the first time since May) do you mind taking a stab at the questions I have above? That'll help me plan a design approach and unblock the issue. If not you, then perhaps @jhugman

@jhugman jhugman moved this from On Deck to Sprint Backlog in Lockwise Mobile Sep 18, 2019
@eliserichards eliserichards moved this from Sprint Backlog to In Progress in Lockwise Mobile Sep 20, 2019
Lockwise Mobile automation moved this from In Progress to Done Oct 14, 2019
@eliserichards eliserichards moved this from Done to QA Ready in Lockwise Mobile Oct 15, 2019
@eliserichards eliserichards added the QA-needed When needing QA Verification/Investigation label Oct 15, 2019
@isabelrios
Copy link
Contributor

Commenting as QA Needed label is added.
I got this working using Douling app and latest master d04e5e3.
But triying with other apps, like Facebook and Twitter I could not get it working.
More testing would be needed so leaving the label here and the issue under the QA column

@abodea
Copy link
Contributor

abodea commented Oct 21, 2019

I was also able to save a login using Duolingo and 9gag app.
I couldn't make it work using: Facebook, Facebook Lite, Instagram, Twitch, Amazon.
Note that Facebook got a save login feature integrated in the app, and maybe because of that, not any other app can save logins on their products.

@sandysage
Copy link

@abodea Unfortunately that has been the case for the autofill feature too. It's something we're considering for how we improve the app going forward: 1. defining some set of sites/apps where autofill is not working 2. triage/scope these cases and 3. come up with a plan for getting all of autofill (both fill and capture) working in more places. I'll create a ticket specifically for that initial definition work.

@abodea
Copy link
Contributor

abodea commented Jan 31, 2020

Verified as fixed on v.4.0.0(6016) using Google Pixel 3 XL(Android 9), Samsung Galaxy S10+(Android 9).
I verified it on the following: Spotify, Facebook, Twitter, Duolingo(App), Twitch, Yahoo, Pinterest.

@abodea abodea added QA-verified For the issues verified by QA and removed QA-needed When needing QA Verification/Investigation labels Jan 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
effort-L Expected to take a sprint for engineering to complete. feature-autofill feature-create feature-CUD priority-P1 QA-verified For the issues verified by QA
Projects
9 participants