Commit 14e6b553 authored by Victor Hugo Vianna Silva's avatar Victor Hugo Vianna Silva Committed by Commit Bot

Fix wrong plaintext password being shown after deletion

This CL fixes the linked bug by ensuring the plaintext password property
in ShowPasswordBehavior is reset when the element is pointed to another
credential, i.e. when "entry" is changed. The result is that removing a
credential, or doing a search filter will hide any plaintext passwords
being shown.

Before what happened was:
- Deleting a pwd would update the savedPasswords array in
  MergePasswordsStoreCopiesBehavior.
- The backing array of the iron-list of pwds in PasswordsSection would
  be updated.
- The "entry" properties of the PasswordListItem in the iron-list would
  be updated *but* the "password" properties wouldn't be updated.
  Contrary to what one may believe, removing an element from the backing
  array of an iron-list doesn't actually remove the corresponding
  element from the DOM.

The CL also makes the visibility state of the password in the edit
dialog not be synced with the corresponding row in the pwd list anymore.

Bug: 1110290
Change-Id: Ibb5d10999b4c966fcf382f5a18e76ce462090188
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2325731
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792681}
parent 7b9d2d5f
...@@ -35,7 +35,6 @@ ...@@ -35,7 +35,6 @@
token-request-manager="[[tokenRequestManager_]]" token-request-manager="[[tokenRequestManager_]]"
</if> </if>
entry="[[activePassword.entry]]" entry="[[activePassword.entry]]"
password="{{activePassword.password}}"
should-show-storage-details= should-show-storage-details=
"[[shouldShowStorageDetails]]"> "[[shouldShowStorageDetails]]">
</password-edit-dialog> </password-edit-dialog>
......
...@@ -26,9 +26,6 @@ export const ShowPasswordBehavior = { ...@@ -26,9 +26,6 @@ export const ShowPasswordBehavior = {
password: { password: {
type: String, type: String,
value: '', value: '',
// If the password is initialized by the parent component, changes in
// visibility should be reflected there too.
notify: true
}, },
// <if expr="chromeos"> // <if expr="chromeos">
...@@ -37,6 +34,15 @@ export const ShowPasswordBehavior = { ...@@ -37,6 +34,15 @@ export const ShowPasswordBehavior = {
// </if> // </if>
}, },
observers: [
'resetPlaintextPasswordOnEntryChanged_(entry)',
],
/** @private */
resetPlaintextPasswordOnEntryChanged_() {
this.password = '';
},
/** /**
* Gets the password input's type. Should be 'text' when password is visible * Gets the password input's type. Should be 'text' when password is visible
* or when there's federated text otherwise 'password'. * or when there's federated text otherwise 'password'.
......
...@@ -297,6 +297,35 @@ suite('PasswordsSection', function() { ...@@ -297,6 +297,35 @@ suite('PasswordsSection', function() {
validatePasswordList(passwordsSection, passwordList); validatePasswordList(passwordsSection, passwordList);
}); });
// Regression test for crbug.com/1110290.
// Test verifies that if the password list is updated, all the plaintext
// passwords are hidden.
test('updatingPasswordListHidesPlaintextPasswords', function() {
const passwordList = [
createPasswordEntry({username: 'user0', id: 0}),
createPasswordEntry({username: 'user1', id: 1}),
];
const passwordsSection = elementFactory.createPasswordsSection(
passwordManager, passwordList, []);
// Make passwords visible.
const passwordListItems =
passwordsSection.root.querySelectorAll('password-list-item');
assertEquals(2, passwordListItems.length);
passwordListItems[0].password = 'pwd0';
passwordListItems[1].password = 'pwd1';
flush();
// Remove first row and verify that the remaining password is hidden.
passwordList.splice(0, 1);
passwordManager.lastCallback.addSavedPasswordListChangedListener(
passwordList);
flush();
assertEquals('', getFirstPasswordListItem(passwordsSection).password);
assertEquals(
'user1', getFirstPasswordListItem(passwordsSection).entry.username);
});
// Test verifies that removing the account copy of a duplicated password will // Test verifies that removing the account copy of a duplicated password will
// still leave the device copy present. // still leave the device copy present.
test('verifyPasswordListRemoveAccountCopy', function() { test('verifyPasswordListRemoveAccountCopy', function() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment