New issue
Advanced search Search tips

Issue 908869 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Manual password saving in password dialog broken

Project Member Reported by battre@chromium.org, Nov 27

Issue description

Go to https://telematici.agenziaentrate.gov.it/Main/login.jsp

Enter the following data into the form:
Username: Username
Password: Password
PIN: 1234

Click on the key icon, observe that you are offered to save Username / 1234.

Change this to Username / Password.

Click save.

Observe that the password form is changed to now contain the content
Username: Username
Password: Password
PIN: Password

 
Cc: vabr@chromium.org
Labels: -Pri-2 Pri-1
https://ebank.esunbank.com.tw/index.jsp is another example. There the password is later filled into a text field. Raising priority to P1.
The problems in both cases are in server predictions. In the first case the server responded that PIN is password, in the second is that the first text field is password and the second text field is username.

I'll implement in the new parser that a text field will be no chosen as password even if the server responses that. We need to have a look why server responded incorrectly and to make overrides for these sites.
Status: Started (was: Assigned)
In both cases, the server predicts the not-so-password field because that's the field users actually save as password. 
https://plx.corp.google.com/scripts2/script_5b._bd2d1e_0000_2633_8f70_001a114a7e5c?p=FORM_SIGNATURE%3A4638058470103394044
https://plx.corp.google.com/scripts2/script_5b._bd2d1e_0000_2633_8f70_001a114a7e5c?p=FORM_SIGNATURE%3A5794880210646308347

Interestingly, users *correct* our fill-on-pageload choices, by overwhelmingly moving their password into the PIN field. With that in mind, I think the explanation is that
1. We save the PIN as password during singup
2. We fill the PIN into the password, because of manual overrides
3. The users move the PIN to the PIN field.
4. The pipeline sees that the PIN field is the place to put saved passwords.

I can't verify the signup flows of these websites, and I don't know how to verify that we have manual overrides in place, therefore I'm guessing for 1 and 2.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31885f533a09743f89c2d90c8a73faa59f82071d

commit 31885f533a09743f89c2d90c8a73faa59f82071d
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Thu Nov 29 10:37:06 2018

Ignore password related predictions for non-password fields.

The server might make mistake and say that <input type=text is
password field. This CL teaches the new parser to ignore such
predictions.

Bug: 908869, 831123
Change-Id: Ib4cfa715e0746b828be3c2356e8ae5bc60717a69
Reviewed-on: https://chromium-review.googlesource.com/c/1354000
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612134}
[modify] https://crrev.com/31885f533a09743f89c2d90c8a73faa59f82071d/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/31885f533a09743f89c2d90c8a73faa59f82071d/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

Cc: -vabr@chromium.org
vabr going hobby only -> reducing involvement.
Please contact me directly in urgent matters.

Sign in to add a comment