New issue
Advanced search Search tips

Issue 658981 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Password Manager confuses duplicate entries between HTTP & HTTPS

Project Member Reported by agektmr@chromium.org, Oct 25 2016

Issue description

Version: Mac Version 56.0.2899.0 canary (64-bit)

1. Store credentials both on http://www.hrs.com and https://www.hrs.com using same id and password
2. 2 credentials seem to be treated separately

- On desktop Chrome chrome://settings/passwords shows obscure "hrs.com"
- On Android settings page, obscure "hrs.com"
- On desktop Chrome with material design settings chrome://settings/managePasswords shows http://www.hrs.com but not https://www.hrs.com

1. Store credentials both on http://www.hrs.com and https://www.hrs.com using different id and password
2. 2 credentials seem to be treated separately

- On desktop Chrome chrome://settings/passwords shows obscure "hrs.com" and shows the one on https://www.hrs.com
- On Android settings page, obscure "hrs.com" and shows the one on https://www.hrs.com
- On desktop Chrome with material design settings chrome://settings/managePasswords shows both http://www.hrs.com one and https://www.hrs.com one

We need valid and consistent behaviors throughout platforms.
 

Comment 1 by battre@chromium.org, Oct 25 2016

Cc: melandory@chromium.org battre@chromium.org
Owner: kolos@chromium.org

Comment 2 by kolos@chromium.org, Oct 25 2016

On chrome://settings/passwords, we merge HTTP and HTTPS entries, because the scheme isn't included in the sort key (https://cs.chromium.org/chromium/src/chrome/browser/ui/passwords/password_manager_presenter.cc?q=createsortkey&sq=package:chromium&l=72). So, if two entries have different schemes, but origin, username and password are the same, the entries would be merged into one entry.

We should discuss what behavior we expect. Shall we merge them or not?


I think not because one can't delete the HTTP credential at all. In the code they are really treated differently.

Comment 4 by kolos@chromium.org, Oct 27 2016

Status: Available (was: Untriaged)
Then we should add the scheme (or secure/insecure flag) to the sort key. 

Comment 5 by vabr@chromium.org, Oct 27 2016

We should not merge HTTP and HTTPS credentials together in any of the UI where the passwords are displayed. Unlike PSL-matched credentials, HTTP vs. HTTPS is a strong boundary which we need to respect, and the UI should indicate so. In particular, as #3 points out, the user needs control of deleting credentials based on scheme.

Comment 6 by vabr@chromium.org, Oct 27 2016

Components: UI>Browser>Passwords
Labels: Hotlist-Polish
Status: Assigned (was: Available)
This issue has currently kolos@ as Owner, but the state is "Available". I'm changing to assigned. Maxim, if you cannot work on this in the near future, please let me know, I'll reassign.

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

I will take it.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 15 2016

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

commit 2064c3247e03ad89688194f7bd4fa79479363644
Author: kolos <kolos@chromium.org>
Date: Tue Nov 15 16:07:36 2016

[Password Manager] Add the scheme to sort keys for chrome://settings/passwords

The scheme was not a part of sort key. Therefore, we merged http and https credentials (if they have the same origin, username and password). See the bug for details.

BUG= 658981 

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

[modify] https://crrev.com/2064c3247e03ad89688194f7bd4fa79479363644/chrome/browser/ui/passwords/password_manager_presenter.cc
[modify] https://crrev.com/2064c3247e03ad89688194f7bd4fa79479363644/chrome/browser/ui/passwords/password_manager_presenter_unittest.cc

Comment 9 by kolos@chromium.org, Dec 12 2016

Status: Fixed (was: Assigned)

Sign in to add a comment