1497682 - LastPass enters password in username field instead of password field

Status: VERIFIED FIXED

(GeckoView :: General, defect, P1)

Description

[Jonathan Almeida [:jonalmeida]]

See focus-android[0] for a more detailed bug report.

Steps to reproduce

    Have an account and login setup on LinkedIn saved in LastPass.
    Go to http://linkedin.com
    Click on the password field.
    Click the popup dialog that shows up from LastPass to auto-fill your password.

Expected behavior

    The username is correctly filled in from LastPass.
    The password is correctly filled in from LastPass.

Actual behavior

    Both, username and password fields are filled with the password.

Device information

    Android device: Pixel 2 (Android 9)
    Focus version: 8.0 (Build #322700813 🦎 62.0.3.20180922023437)


[0]: https://github.com/mozilla-mobile/focus-android/issues/3501#issuecomment-428346024

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

Jonathan can you test this again when you pick up GeckoView 63?

Comment 2

[Chris Peterson [:cpeterson]]

We expect LastPass to be fixed in Focus (by bug 1330257) after they pick up GV 63 next week.

Comment 3

[Jonathan Almeida [:jonalmeida]]

David & Chris, I just tested this on Focus with GV 63 and I'm still able to reproduce the issue.

I applied this PR[0] which Focus is going to be merging next week and verified I was using GV by checking the user-agent[1].

NI cpeterson in case you want to prioritize this.

[0]: https://github.com/mozilla-mobile/focus-android/pull/3619
[1]: https://whatismybrowser.com

Comment 4

[Chris Peterson [:cpeterson]]

@ Jonathan, thanks for the testing GV 63! Do you know offhand if this problem happens in Fennec, too?

@ Jim, can you take a look at this LastPass bug next week? Entering the password in the username field would make LastPass unusable. This sounds like a bug we should to fix in GV 63 for Focus 8.0.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Does it reproduce on other sites? IMO this would be a lower priority if it's only LinkedIn.

Comment 6

[Jonathan Almeida [:jonalmeida]]

I've been able to repro this on a several websites including Google SSO.

Comment 7

[Chris Peterson [:cpeterson]]

Unassigning Jim. We'll need to find a new owner for this bug. This bug is probably a release blocker for testing GV 64 with Focus 8.0 prod users.

Comment 8

[Chris Peterson [:cpeterson]]

64=fix-optional because this bug is (very!) nice-to-have but doesn't block Focus 8.0.

Comment 9

[Makoto Kato [:m_kato]]

Attachment: Bug 1497682 - Part 1. Add API to recognize username field and password field from a field in form. r?MattN

I would like to get username fields etc from form or input element in form.

Since LoginManagerContent._getFormFields has "_" prefix (it means private
member) and LoginManagerCotnent.getFieldContext doesn't return elements,
I would like to add simple API for it.

Also, it doesn't have FormLikes parameter, I cannot write unit test like
getFieldContext.  Unit test will be followed by GeckoView junit.

Comment 10

[Makoto Kato [:m_kato]]

Attachment: Bug 1497682 - Part 2. Don't set inputType to all <input type=text>. r?droeh

LastPass will fill password to all input elements which InputType is
TYPE_CALSS_TEXT and TYPE_TEXT_VARIATION_WEB_EDIT_TEXT and has no AutofillHint.
And it will fill username when InputType and AutofillHint is nothing in
<input type="text">.

Actually, current implementation of GeckoView sets InputType only for
<input type="text">, so LastPass fills password to all <input type="text">

So as workaround, we should set InputType and AutofillHint when input element
presumes username fields.

Depends on D12880

Comment 11

[Makoto Kato [:m_kato]]

Attachment: Bug 1497682 - Part 3. Add junit test. r?droeh

Add autofill hint test if using Android 8+.

Depends on D12881

Comment 12

[Chris Peterson [:cpeterson]]

In anticipation of the creation of the GECKOVIEW_64_RELBRANCH next week, I am moving this bug from status-firefox64=affected to status-geckoview64=affected.

Comment 13

[Chris Peterson [:cpeterson]]

Makoto, do you have any updates on this LastPass bug? It looks like patch #2 and #3 have r+ and patch #1 needs to address some review comments?

Comment 14

[Makoto Kato [:m_kato]]

(In reply to Chris Peterson [:cpeterson] from comment #13)
> Makoto, do you have any updates on this LastPass bug? It looks like patch #2
> and #3 have r+ and patch #1 needs to address some review comments?

I am working part 1 fix.  I will send review soon.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5727b563004
Part 1. Add API to recognize username field and password field from a field in form. r=MattN
https://hg.mozilla.org/integration/mozilla-inbound/rev/78488dedd35e
Part 2. Don't set inputType to all <input type=text>. r=droeh
https://hg.mozilla.org/integration/mozilla-inbound/rev/b68c8690cc9e
Part 3. Add junit test. r=droeh

Comment 16

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/e5727b563004
https://hg.mozilla.org/mozilla-central/rev/78488dedd35e
https://hg.mozilla.org/mozilla-central/rev/b68c8690cc9e

Comment 17

[Chris Peterson [:cpeterson]]

Makoto, should we uplift your LastPass fix to Fennec and GV 65 Beta (or even the GV 64 relbranch used in Focus 8.0)?

Comment 18

[Makoto Kato [:m_kato]]

Attachment: rebasing for beta: Bug 1497682 - Part 1. Add API to recognize username field and password field from a field in form. r=MattN

Comment 19

[Makoto Kato [:m_kato]]

Comment on attachment 9033308 [details] [diff] [review]
rebasing for beta: Bug 1497682 - Part 1. Add API to recognize username field and password field from a field in form. r=MattN

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1330257

User impact if declined: By bug 1330257, Firefox Android (and GeckoView) supports autofill framework that is a new feature on Android 8+.   When using LastPass (a famous passoword manager) for Android, LastPass fills password to all text fields that has no password type attribute.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 1. Have an account and login setup on LinkedIn saved in LastPass.
2. Go to http://linkedin.com
3. Click on the password field.
4. Click the popup dialog that shows up from LastPass to auto-fill your password.

AR
Both, username and password fields are filled with the password.

ER
Don't fill password on non-password field.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Before this fix, we set a hint to all text fields as generic text (if no number type etc).  But it causes this issue on LastPass unfortunately.

Firefox desktop (autofill JS module) has login field detection for username ans password fields.  So this fix uses this desktop's Javascript module to detect username field and password field, then we set a valid hint to autofill framework for username and password correctly.

String changes made/needed:

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9033308 [details] [diff] [review]
rebasing for beta: Bug 1497682 - Part 1. Add API to recognize username field and password field from a field in form. r=MattN

[Triage Comment]
Important compat fix for LastPass users. Makes me a bit nervous, but we're still early enough in the beta cycle that we should be able to notice any regressions in time. Let's have QA give this a solid pass as well. Thanks for including an automated test. Approved for 65.0b7.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b65f3b9a581a
https://hg.mozilla.org/releases/mozilla-beta/rev/27b291b4e078
https://hg.mozilla.org/releases/mozilla-beta/rev/f8cd4d3f5888

Comment 22

[Makoto Kato [:m_kato]]

Attachment: Rebasing for GV64: Bug 1497682 - Part 3. Add junit test. r=droeh

Comment 23

[Andrei Bodea[:andrei]]

Device:
 - Google Pixel C(tab) (Android 8.0.0)
 - Google Pixel XL (Android 9.0.0 / P)
 - Google Pixel 3 XL(Android 9.0.0 / P)
 - Samsung Galaxy Note 9 (Android 8.1.0)

Verified as fixed in the latest Beta build 65b.0b7 and the latest Nightly Build 66.0a1(07-Jan-2019).
Note that I verified the login at least 5 times for each device and it worked every time, the username and password were assigned in the right field.