New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 602195 link

Starred by 8 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Ignore element names in password blacklisting

Project Member Reported by vabr@chromium.org, Apr 11 2016

Issue description

Currently, username_element and password_element are used for matching a saved blacklist entry to an observed password form.

As a result, on pages which use different element names each time a login page is displayed, the user cannot blacklist a credential.

We should ignore element names in the matching of blacklisted forms.
 

Comment 1 by vabr@chromium.org, Apr 11 2016

Owner: cfroussios@chromium.org
Status: Assigned (was: Available)
The key is to get PasswordFormManager::IsBlacklisted return true even if PasswordForm::username_element and PasswordForm::password_element do not match between the stored blacklisted and observed forms.

Comment 2 by battre@chromium.org, Apr 11 2016

Keep backwards compatibility in mind...

Comment 3 by vabr@chromium.org, Apr 12 2016

You mean use-cases where the users would have two similar forms on the same page, and used blacklisting to only save passwords on one of them?

For this to happen, those forms would need to share the action URL and origin (including path), which makes that situation rather unlikely.

Comment 4 by battre@chromium.org, Apr 12 2016

I just mean that users may have blacklisted forms in the past and these should stay blacklisted.

Comment 5 by vabr@chromium.org, Apr 12 2016

Yes, this change increases the set of blacklisted forms, what has been blacklisted will stay blacklisted.

Comment 6 by vabr@chromium.org, Apr 12 2016

(Just for completeness, my answer in #3 was not correct, because blacklisting credentials do not go through scoring, they are applied separately. The only remaining matching done is then the same origin requirement, nothing about actions. But the conclusion is still that it is unlikely that this would suddenly blacklist a form which the user wanted not to have blacklisted. And #5 is still correct in saying that whatever was blacklisted before will stay blacklisted.)
If we ignore every aspect of the form itself, we will essentially be putting the entire site in the blacklist. As vabr@ pointed out, if we falsely identify a form as a password form, the user will likely blacklist it and unknowingly blacklist the entire site.

Perhaps it would make sense, instead of removing these conditions, to add a rule that a form also matches the blacklist if it has the same action. The idea is that the recipient of the form is less likely to be a moving target as well.

Comment 8 by vabr@chromium.org, Apr 12 2016

#7 raises a good point; the "same origin" check mentioned in #6 is indeed without path, so it would cause blacklisting of the whole domain. That might break the user experience as described in the second sentence of #7.

I just checked a random site with this issue, and it encodes the session ID in the action path, though, so we might need to think of another way to distinguish the forms to blacklist.

Comment 9 by vabr@chromium.org, Apr 12 2016

(The site I checked was financepilot-pe.mlp.de.)

Comment 10 by vabr@chromium.org, Apr 13 2016

vasilii@ mentioned that this issue does not occur when the "smart bubble" is used. See bug 564045 for that feature and its design doc. It might be a good exercise to understand why the smart bubble solves this.
To force the smart bubble behaviour, one can use a command-line flag, e.g.,
--force-fieldtrial-params="PasswordSmartBubble.3-Times:dismissal_count/3"

Depending on the findings, we might either get inspiration for the current blacklisting mechanism, or decide to give up and wait until the "smart bubble" is fully launched.
I was able to verify that smart bubble will eventually blacklist the form. The reason for this is that it works with the general page and doesn't try to identify the specific form in the page.

We can combine this with the existing conditions: a form is blacklisted if
it matches a specific form
or
there are already multiple blacklisted forms in this page.
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 14 2016

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

commit a4a7c52f6f98adf2aa9fc752c0b6e579a54476b0
Author: cfroussios <cfroussios@chromium.org>
Date: Thu Apr 14 14:02:52 2016

Blacklisting applied to the form and the page

A form is blacklisted if:
It has the same elements as a blacklisted form from the same site
OR
It appears on the same page as a blacklisted form from the same site

BUG= 602195 

Review URL: https://codereview.chromium.org/1886283002

Cr-Commit-Position: refs/heads/master@{#387303}

[modify] https://crrev.com/a4a7c52f6f98adf2aa9fc752c0b6e579a54476b0/components/password_manager/core/browser/password_form_manager.cc
[modify] https://crrev.com/a4a7c52f6f98adf2aa9fc752c0b6e579a54476b0/components/password_manager/core/browser/password_form_manager_unittest.cc

Status: Fixed (was: Assigned)

Comment 14 by vabr@chromium.org, Jun 23 2016

Cc: vabr@chromium.org
 Issue 622572  has been merged into this issue.

Comment 15 by vabr@chromium.org, Jul 7 2016

Cc: gcasto@chromium.org tedc...@chromium.org
 Issue 497594  has been merged into this issue.
Cc: -vabr@chromium.org

Sign in to add a comment