New issue
Advanced search Search tips

Issue 604750 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 400674



Sign in to add a comment

Saved credential information doesn't match with what actually been stored

Project Member Reported by agektmr@chromium.org, Apr 19 2016

Issue description

Version: Version 52.0.2707.0 canary (64-bit)
OS: Mac OS X 10.11.4

What steps will reproduce the problem?
(1) Go to https://credential-management-sample.appspot.com/
(2) Use all kinds of credentials and store them: id/password, google sign-in, facebook login
(3) See key icon inside address bar and chrome://settings/passwords

What is the expected output?
list of id/password, google sign-in and facebook login

What do you see instead?
Only google sign-in and facebook login, or id/password and google sign-in, etc
I haven't found rules but I only see 2 out of 3. Maybe there are more complex patterns.


Please use labels and text to provide additional information.

 
Cc: vabr@chromium.org vasi...@chromium.org
Labels: -Pri-3 Pri-1
Status: Assigned (was: Untriaged)
For me even the saving of federated credentials didn't trigger in the first place. Could someone please take a look? Thanks!
Cc: battre@chromium.org

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

Labels: Needs-Feedback
Please specify concrete steps and scenarios. The key icon has different meaning in different steps of signing in and out.

sabineb@ -- which federated credentials did you try, Google or Facebook? With Facebook all seems to work OK. With Google, the app says it's logged in, but the JavaScript console says "Failed to construct 'FederatedCredential': '/images/default_img.png' is not a valid URL". No credential is saved in that case, and hence Chrome does not pop up any UI.
I've just sorted the issue vabr@ encountered: "Failed to construct 'FederatedCredential': '/images/default_img.png' is not a valid URL"

The step is already described. Just try sign in using all options - id/password, Google Sign-In and Facebook Login.
All I see is 2 credentials in chrome://settings/passwords although I see 3 credentials in key icon bubble. Images attached.
chrome_settings_passwords.png
48.6 KB View Download
key_icon_bubble.png
48.8 KB View Download
Owner: kolos@chromium.org
This is a UI bug in chrome://settings/passwords. All the credentials are in the password store. However, different federations hide each other in the list.
Blocking: 400674
Components: UI>Browser>Passwords
Labels: -Needs-Feedback

Comment 7 by kolos@chromium.org, Apr 28 2016

Status: Started (was: Assigned)

Comment 8 by kolos@chromium.org, Apr 28 2016

We hide duplicates in the password list based on origin, username and password (i.e. if all of these are equal for some entries, we hide all of the entries except one). The problem is that we don't count federations (i.e. entries with the same origin, username and password are collapsed to one entry even if federations are different). 

The federation should be included into sorting (grouping) key.
Labels: M-51

Comment 10 by kolos@chromium.org, Apr 28 2016

agektmr@: great thanks for reporting this!
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 28 2016

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

commit 7c82fec6686c0ee3b38bb70d86d0f0c1837127c9
Author: kolos <kolos@chromium.org>
Date: Thu Apr 28 20:12:17 2016

[Password Manager] Add federations to sort key on password page

If federations are not included into sort key, entries with the same origin, username and password, but different federation will be collapsed into one entry. That is not correct.

BUG= 604750 

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

[modify] https://crrev.com/7c82fec6686c0ee3b38bb70d86d0f0c1837127c9/chrome/browser/ui/passwords/password_manager_presenter.cc
[modify] https://crrev.com/7c82fec6686c0ee3b38bb70d86d0f0c1837127c9/chrome/browser/ui/passwords/password_manager_presenter.h
[modify] https://crrev.com/7c82fec6686c0ee3b38bb70d86d0f0c1837127c9/chrome/browser/ui/passwords/password_manager_presenter_unittest.cc

Comment 12 by kolos@chromium.org, Apr 29 2016

Status: Fixed (was: Started)
Sabine, should it be merged back?
Thanks for checking, Vasilii. Yes, the fix should please be merged back. 

We do have a bunch of partners who are in the process of integrating the API and who will go live in the M51 timeframe. Let me know if I should talk to the release managers. 
Labels: Merge-Request-51
Status: Started (was: Fixed)
We'd like to merge r390457 to M51. The UI change is innocuous.

Comment 16 by tin...@google.com, May 2 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 17 by bugdroid1@chromium.org, May 2 2016

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

commit 3877e40f5ac8fc8a45aabab1be84a2472abaadff
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Mon May 02 11:33:00 2016

[Password Manager] Add federations to sort key on password page

If federations are not included into sort key, entries with the same origin, username and password, but different federation will be collapsed into one entry. That is not correct.

BUG= 604750 

Review-Url: https://codereview.chromium.org/1927753003
Cr-Commit-Position: refs/heads/master@{#390457}
(cherry picked from commit 7c82fec6686c0ee3b38bb70d86d0f0c1837127c9)

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

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

[modify] https://crrev.com/3877e40f5ac8fc8a45aabab1be84a2472abaadff/chrome/browser/ui/passwords/password_manager_presenter.cc
[modify] https://crrev.com/3877e40f5ac8fc8a45aabab1be84a2472abaadff/chrome/browser/ui/passwords/password_manager_presenter.h
[modify] https://crrev.com/3877e40f5ac8fc8a45aabab1be84a2472abaadff/chrome/browser/ui/passwords/password_manager_presenter_unittest.cc

Status: Fixed (was: Started)
Labels: TE-Verified-51.0.2704.36 TE-Verified-M51
Tested the issue on Mac 10.10.5 using 51.0.2704.36 as per steps in comment #0.All 3(list of id/password, google sign-in and facebook login) are seen.
Please find attached screenshots.

Marking it as TE-Verified.
604750_settings.png
554 KB View Download
604750_address bar.png
367 KB View Download
Cc: -vabr@chromium.org

Sign in to add a comment