New issue
Advanced search Search tips

Issue 723679 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Chrome Password Manager doesn't fill on accounts.google.com

Project Member Reported by dvadym@chromium.org, May 17 2017

Issue description

After recent changes in accounts.google.com UI, Chrome Password Manager starts to be flaky in filling saved password on accounts.google.com.

The reason is that JavaScript changes url (path in url, not domain) between Password Manager discovers a password form and when fill data is received from Password Store. And filling-not-filling depends whether JavaScript or Password Manager are faster.

The fix is to compare only domains. This is more correct behavior, since look-up in the password store is done by domain anyway.

 
Project Member

Comment 1 by bugdroid1@chromium.org, May 18 2017

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

commit 0273d9bd3e2d25f1485ab7a9469318c74ab7cb17
Author: dvadym <dvadym@chromium.org>
Date: Thu May 18 16:34:29 2017

[Password Manager] Make filling robust against changing url by JavaScript.

When PasswordAutofillAgent receives filling data from the browser, it checks that origin of this data is the same of the current frame origin. If JavaScript changes origin between PasswordAutofillAgent discovers a password form and when it receives filling, filling fails.

This CL replaces checking by origin to by checking by signon_realm (signon_realm=scheme:origin:port), which is a primary key for retrieving credentials from the store, so it doesn't change any security guarantees.

BUG= 723679 

Review-Url: https://codereview.chromium.org/2893633002
Cr-Commit-Position: refs/heads/master@{#472839}

[modify] https://crrev.com/0273d9bd3e2d25f1485ab7a9469318c74ab7cb17/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/0273d9bd3e2d25f1485ab7a9469318c74ab7cb17/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/0273d9bd3e2d25f1485ab7a9469318c74ab7cb17/components/autofill/content/renderer/password_form_conversion_utils.cc
[modify] https://crrev.com/0273d9bd3e2d25f1485ab7a9469318c74ab7cb17/components/autofill/content/renderer/password_form_conversion_utils.h

Comment 2 by dvadym@chromium.org, May 22 2017

Labels: Merge-Request-59
This is a bugfix for filling saved passwords on accounts.google.com (which is broken after recent changes in accounts.google.com UI). This CL is very simple and it's very unlikely that it can break anything.
Project Member

Comment 3 by sheriffbot@chromium.org, May 22 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 4 by bugdroid1@chromium.org, May 22 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3454ffaef5cb228f599efe8158bd1bc41fb479eb

commit 3454ffaef5cb228f599efe8158bd1bc41fb479eb
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Mon May 22 12:25:10 2017

[Password Manager, merge to M-59] Make filling robust against changing url by JavaScript.

When PasswordAutofillAgent receives filling data from the browser, it checks that origin of this data is the same of the current frame origin. If JavaScript changes origin between PasswordAutofillAgent discovers a password form and when it receives filling, filling fails.

This CL replaces checking by origin to by checking by signon_realm (signon_realm=scheme:origin:port), which is a primary key for retrieving credentials from the store, so it doesn't change any security guarantees.

TBR=kolos@chromium.org

BUG= 723679 

Review-Url: https://codereview.chromium.org/2893633002
Cr-Original-Commit-Position: refs/heads/master@{#472839}
Review-Url: https://codereview.chromium.org/2900713003 .
Cr-Commit-Position: refs/branch-heads/3071@{#648}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/3454ffaef5cb228f599efe8158bd1bc41fb479eb/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/3454ffaef5cb228f599efe8158bd1bc41fb479eb/components/autofill/content/renderer/password_autofill_agent.cc
[modify] https://crrev.com/3454ffaef5cb228f599efe8158bd1bc41fb479eb/components/autofill/content/renderer/password_form_conversion_utils.cc
[modify] https://crrev.com/3454ffaef5cb228f599efe8158bd1bc41fb479eb/components/autofill/content/renderer/password_form_conversion_utils.h

Comment 5 by dvadym@chromium.org, May 29 2017

Status: Fixed (was: Started)

Sign in to add a comment