New issue
Advanced search Search tips

Issue 610328 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 400674



Sign in to add a comment

Credential Manager API competes with autofill password manager on updating

Project Member Reported by vasi...@chromium.org, May 9 2016

Issue description

Version: 52
OS: all

What steps will reproduce the problem?
(1) Visit any site that integrated the Credential Manager API.
(2) Save one credential via the API.
(3) Login with that credential typing into the form.

The credential manager API updates the credential - correct.
The autofill password manager overwrites it - wrong. The obvious side effect is that the credential becomes non zero click one.

Repeating the steps on top of that state produces even more clutter.
The credential manager API silently saves the previous credential - correct.
The autofill password manager updates the previous one creating a duplicate.
The issue was reproduced by LinkedIn.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 9 2016

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

commit f0a4e30ea6dd8a17ff2fda49239037452e4b44b1
Author: vasilii <vasilii@chromium.org>
Date: Mon May 09 16:48:54 2016

Kill the autofill password manager in case 'store()' was called.

The previous CL (https://codereview.chromium.org/1929953002) kinda did the same thing but only for saving a credential. At the same time it shouldn't be any different from silently updating the existing one.
Once the site calls 'store()' it demonstrates that it posses a valid credential. Therefore, the autofill password manager shouldn't intervene.

BUG= 610328 

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

[modify] https://crrev.com/f0a4e30ea6dd8a17ff2fda49239037452e4b44b1/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/f0a4e30ea6dd8a17ff2fda49239037452e4b44b1/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/f0a4e30ea6dd8a17ff2fda49239037452e4b44b1/chrome/browser/password_manager/credential_manager_browsertest.cc
[modify] https://crrev.com/f0a4e30ea6dd8a17ff2fda49239037452e4b44b1/components/password_manager/content/browser/credential_manager_impl.cc
[modify] https://crrev.com/f0a4e30ea6dd8a17ff2fda49239037452e4b44b1/components/password_manager/content/browser/credential_manager_impl_unittest.cc
[modify] https://crrev.com/f0a4e30ea6dd8a17ff2fda49239037452e4b44b1/components/password_manager/core/browser/password_manager_client.h
[modify] https://crrev.com/f0a4e30ea6dd8a17ff2fda49239037452e4b44b1/components/password_manager/core/browser/stub_password_manager_client.cc
[modify] https://crrev.com/f0a4e30ea6dd8a17ff2fda49239037452e4b44b1/components/password_manager/core/browser/stub_password_manager_client.h
[modify] https://crrev.com/f0a4e30ea6dd8a17ff2fda49239037452e4b44b1/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h
[modify] https://crrev.com/f0a4e30ea6dd8a17ff2fda49239037452e4b44b1/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm

Labels: Merge-Request-51 M-51 OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
I want to merge r392337. The fix is important for the Credential Manager API launch. The risk of something going wrong is minimal.

Comment 3 by tin...@google.com, May 10 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Project Member

Comment 4 by bugdroid1@chromium.org, May 10 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/249dd91407d778daed55e2a3a4abedb9e69ff527

commit 249dd91407d778daed55e2a3a4abedb9e69ff527
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Tue May 10 10:49:17 2016

Kill the autofill password manager in case 'store()' was called.

The previous CL (https://codereview.chromium.org/1929953002) kinda did the same thing but only for saving a credential. At the same time it shouldn't be any different from silently updating the existing one.
Once the site calls 'store()' it demonstrates that it posses a valid credential. Therefore, the autofill password manager shouldn't intervene.

BUG= 610328 

Review-Url: https://codereview.chromium.org/1962713002
Cr-Commit-Position: refs/heads/master@{#392337}
(cherry picked from commit f0a4e30ea6dd8a17ff2fda49239037452e4b44b1)

R=vabr@chromium.org

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

Cr-Commit-Position: refs/branch-heads/2704@{#465}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/249dd91407d778daed55e2a3a4abedb9e69ff527/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/249dd91407d778daed55e2a3a4abedb9e69ff527/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/249dd91407d778daed55e2a3a4abedb9e69ff527/chrome/browser/password_manager/credential_manager_browsertest.cc
[modify] https://crrev.com/249dd91407d778daed55e2a3a4abedb9e69ff527/components/password_manager/content/browser/credential_manager_dispatcher.cc
[modify] https://crrev.com/249dd91407d778daed55e2a3a4abedb9e69ff527/components/password_manager/content/browser/credential_manager_dispatcher_unittest.cc
[modify] https://crrev.com/249dd91407d778daed55e2a3a4abedb9e69ff527/components/password_manager/core/browser/password_manager_client.h
[modify] https://crrev.com/249dd91407d778daed55e2a3a4abedb9e69ff527/components/password_manager/core/browser/stub_password_manager_client.cc
[modify] https://crrev.com/249dd91407d778daed55e2a3a4abedb9e69ff527/components/password_manager/core/browser/stub_password_manager_client.h
[modify] https://crrev.com/249dd91407d778daed55e2a3a4abedb9e69ff527/ios/chrome/browser/passwords/ios_chrome_password_manager_client.h
[modify] https://crrev.com/249dd91407d778daed55e2a3a4abedb9e69ff527/ios/chrome/browser/passwords/ios_chrome_password_manager_client.mm

Status: Fixed (was: Started)

Sign in to add a comment