Show Form-Not-Secure warning for username fields even if there is no stored data |
|||
Issue descriptionCurrently, we show the Form-Not-Secure warning for fields in password forms if: - there is stored autofill data for the field - or the field is a password field - or the field has a 'username' autocomplete attribute. We'd like to show the warning for all username fields even when there is no stored data, but the password manager code isn't structured to make that easy. When we add the Form-Not-Secure warning to a field without stored data, we don't know whether the field is a username field. [1] I *think* we can fix this without too much trouble as follows. In PasswordAutofillAgent::ShowSuggestions(), if the form is on a nonsecure page and we don't find stored autofill data, then call CreatePasswordFormFromWebForm [2] on the element's form(). The returned PasswordForm will contain the name of a predicted username element, which we can compare to the element on which ShowSuggestions() is being called, and show the Form-Not-Secure warning if it matches. [1] https://cs.chromium.org/chromium/src/components/autofill/content/renderer/password_autofill_agent.cc?q=PasswordAutofillAgent::ShowS&sq=package:chromium&l=853 [2] https://cs.chromium.org/chromium/src/components/autofill/content/renderer/password_form_conversion_utils.cc?sq=package:chromium&l=668
,
Feb 4 2017
ShowSuggestions() gets called a lot (e.g. every keystroke) and CreatePasswordFormFromWebForm looks pretty expensive. Is there a good place to cache the "Is a username field" result to avoid redundant analysis? Elsewhere, we're already keeping track of whether the page has a visible password input at all (to light up the omnibox warning)-- can we restrict the new logic to cases where we know the page has a password field?
,
Feb 4 2017
elawrence made another suggestion off-thread that we could just put the warning on any element in a form in which there is a password field. That seems very nice and simple, so I think we should do that.
,
Feb 6 2017
,
Feb 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e4310b09c5b49b6ce2825021919049704a8ff4a6 commit e4310b09c5b49b6ce2825021919049704a8ff4a6 Author: elawrence <elawrence@chromium.org> Date: Thu Feb 16 18:47:29 2017 Show Login Not Secure on all form fields if a password field is visible Previously, the "Login Not Secure" notice was only shown for fields in forms if the input type was Password or if the text field contained an AutoComplete="username" attribute. This change enhances the warning to show "Login Not Secure" notice for any text field in a form containing a visible password field. BUG= 688612 TEST=Enable FNS, visit http://webdbg.com/test/forms/findlogins.htm Review-Url: https://codereview.chromium.org/2682473002 Cr-Commit-Position: refs/heads/master@{#451025} [modify] https://crrev.com/e4310b09c5b49b6ce2825021919049704a8ff4a6/chrome/browser/password_manager/password_manager_browsertest.cc [modify] https://crrev.com/e4310b09c5b49b6ce2825021919049704a8ff4a6/chrome/test/data/password/password_form.html [modify] https://crrev.com/e4310b09c5b49b6ce2825021919049704a8ff4a6/components/autofill/content/renderer/password_autofill_agent.cc [modify] https://crrev.com/e4310b09c5b49b6ce2825021919049704a8ff4a6/components/autofill/content/renderer/password_autofill_agent.h
,
Feb 16 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by est...@chromium.org
, Feb 4 2017