Commit fda39078 authored by Viktor Semeniuk's avatar Viktor Semeniuk Committed by Commit Bot

[Passwords] Username editing UI

This change adds possibility to edit username in Settings > Passwords.
If edited username is already used for the same website error message
is shown.

Bug: 377410

Change-Id: Ifbd45f459e2d6731f8e5fd6bf7df069efd6999ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424315
Commit-Queue: Viktor Semeniuk <vsemeniuk@google.com>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811224}
parent 5078e039
...@@ -482,6 +482,9 @@ ...@@ -482,6 +482,9 @@
<message name="IDS_SETTINGS_PASSWORD_EDIT_FOOTNOTE" desc="A footnote for the dialog which allows the user to edit a saved password."> <message name="IDS_SETTINGS_PASSWORD_EDIT_FOOTNOTE" desc="A footnote for the dialog which allows the user to edit a saved password.">
Make sure the password you are saving matches your password for <ph name="WEBSITE">$1<ex>airbnb.com</ex></ph> Make sure the password you are saving matches your password for <ph name="WEBSITE">$1<ex>airbnb.com</ex></ph>
</message> </message>
<message name="IDS_SETTINGS_PASSWORD_USERNAME_ALREADY_USED" desc="An error message when user tried editing the username to a value which is already used for the same site.">
You already saved this username for this site
</message>
<message name="IDS_SETTINGS_PASSWORD_COPY" desc="Label for a context menu item that allows to copy the selected password into clipboard."> <message name="IDS_SETTINGS_PASSWORD_COPY" desc="Label for a context menu item that allows to copy the selected password into clipboard.">
Copy password Copy password
</message> </message>
......
65b985cbdd9905b01170b331f7d9ce3b19e60d90
\ No newline at end of file
<style include="settings-shared passwords-shared"> <style include="settings-shared passwords-shared">
cr-input {
--cr-input-error-display: none;
}
cr-input:not(:last-of-type) { cr-input:not(:last-of-type) {
margin-bottom: var(--cr-form-field-bottom-spacing); margin-bottom: var(--cr-form-field-bottom-spacing);
} }
...@@ -27,14 +23,15 @@ ...@@ -27,14 +23,15 @@
value="[[entry.urls.link]]" on-blur="onInputBlur_" readonly> value="[[entry.urls.link]]" on-blur="onInputBlur_" readonly>
</cr-input> </cr-input>
<cr-input id="usernameInput" label="$i18n{editPasswordUsernameLabel}" <cr-input id="usernameInput" label="$i18n{editPasswordUsernameLabel}"
value="[[entry.username]]" on-blur="onInputBlur_" readonly> readonly="[[!isEditDialog_]]" invalid="[[usernameInputInvalid_]]"
on-value-changed="validateUsername_" value="[[entry.username]]"
error-message="$i18n{usernameAlreadyUsed}">
</cr-input> </cr-input>
<cr-input id="passwordInput" label="$i18n{editPasswordPasswordLabel}" <cr-input id="passwordInput" label="$i18n{editPasswordPasswordLabel}"
type="[[getPasswordInputType_(isPasswordVisible_, entry.password)]]" type="[[getPasswordInputType_(isPasswordVisible_, entry.password)]]"
value="[[getPassword_(entry.password)]]" on-blur="onInputBlur_" value="[[getPassword_(entry.password)]]" class="password-input"
class="password-input" readonly="[[!isEditDialog_]]" readonly="[[!isEditDialog_]]" invalid="{{passwordInputInvalid_}}"
required="[[isEditDialog_]]" auto-validate="[[isEditDialog_]]" required="[[isEditDialog_]]" auto-validate="[[isEditDialog_]]">
invalid="{{inputInvalid_}}">
<cr-icon-button id="showPasswordButton" <cr-icon-button id="showPasswordButton"
class$="[[getIconClass_(isPasswordVisible_, entry.password)]]" class$="[[getIconClass_(isPasswordVisible_, entry.password)]]"
slot="suffix" hidden$="[[entry.federationText]]" slot="suffix" hidden$="[[entry.federationText]]"
...@@ -54,7 +51,7 @@ ...@@ -54,7 +51,7 @@
$i18n{cancel} $i18n{cancel}
</cr-button> </cr-button>
<cr-button id="actionButton" class="action-button" <cr-button id="actionButton" class="action-button"
on-click="onActionButtonTap_" disabled="[[inputInvalid_]]"> on-click="onActionButtonTap_" disabled="[[isSaveButtonDisabled_]]">
[[getActionButtonName_(isEditDialog_)]] [[getActionButtonName_(isEditDialog_)]]
</cr-button> </cr-button>
</div> </div>
......
...@@ -22,6 +22,7 @@ import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js'; ...@@ -22,6 +22,7 @@ import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js';
import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {loadTimeData} from '../i18n_setup.js'; import {loadTimeData} from '../i18n_setup.js';
import {MultiStorePasswordUiEntry} from './multi_store_password_ui_entry.js';
import {PasswordManagerImpl} from './password_manager_proxy.js'; import {PasswordManagerImpl} from './password_manager_proxy.js';
import {ShowPasswordBehavior} from './show_password_behavior.js'; import {ShowPasswordBehavior} from './show_password_behavior.js';
...@@ -36,6 +37,27 @@ Polymer({ ...@@ -36,6 +37,27 @@ Polymer({
properties: { properties: {
shouldShowStorageDetails: {type: Boolean, value: false}, shouldShowStorageDetails: {type: Boolean, value: false},
/**
* Saved passwords after deduplicating versions that are repeated in the
* account and on the device.
* @type {!Array<!MultiStorePasswordUiEntry>}
*/
savedPasswords: {
type: Array,
value: () => [],
},
/**
* Usernames for the same website. Used for the fast check whether edited
* username is already used.
* @private {?Set<string>}
*/
usernamesForSameOrigin: {
type: Object,
value: null,
},
/** @private */ /** @private */
editPasswordsInSettings_: { editPasswordsInSettings_: {
type: Boolean, type: Boolean,
...@@ -64,15 +86,36 @@ Polymer({ ...@@ -64,15 +86,36 @@ Polymer({
}, },
/** /**
* Whether the input is invalid. * Whether the username input is invalid.
* @private * @private
*/ */
inputInvalid_: Boolean, usernameInputInvalid_: Boolean,
/**
* Whether the password input is invalid.
* @private
*/
passwordInputInvalid_: Boolean,
/**
* If either username or password entered incorrectly the save button will
* be disabled.
* @private
* */
isSaveButtonDisabled_: {
type: Boolean,
computed:
'computeIsSaveButtonDisabled_(usernameInputInvalid_, passwordInputInvalid_)'
}
}, },
/** @override */ /** @override */
attached() { attached() {
this.$.dialog.showModal(); this.$.dialog.showModal();
this.usernamesForSameOrigin =
new Set(this.savedPasswords
.filter(item => item.urls.shown === this.entry.urls.shown)
.map(item => item.username));
}, },
/** /**
...@@ -241,5 +284,29 @@ Polymer({ ...@@ -241,5 +284,29 @@ Polymer({
*/ */
getFootnote_() { getFootnote_() {
return this.i18n('editPasswordFootnote', this.entry.urls.shown); return this.i18n('editPasswordFootnote', this.entry.urls.shown);
} },
/**
* Helper function that checks if save button should be disabled.
* @return {boolean}
* @private
*/
computeIsSaveButtonDisabled_() {
return this.usernameInputInvalid_ || this.passwordInputInvalid_;
},
/**
* Helper function that checks whether edited username is not used for the
* same website.
* @private
*/
validateUsername_() {
if (this.entry.username !== this.$.usernameInput.value) {
this.usernameInputInvalid_ =
this.usernamesForSameOrigin.has(this.$.usernameInput.value);
} else {
this.usernameInputInvalid_ = false;
}
},
}); });
...@@ -34,9 +34,8 @@ ...@@ -34,9 +34,8 @@
<if expr="chromeos"> <if expr="chromeos">
token-request-manager="[[tokenRequestManager]]" token-request-manager="[[tokenRequestManager]]"
</if> </if>
entry="[[activePassword.entry]]" entry="[[activePassword.entry]]" saved-passwords="[[savedPasswords]]"
should-show-storage-details= should-show-storage-details="[[shouldShowStorageDetails]]">
"[[shouldShowStorageDetails]]">
</password-edit-dialog> </password-edit-dialog>
</template> </template>
......
...@@ -28,6 +28,7 @@ import {SyncBrowserProxyImpl} from '../people_page/sync_browser_proxy.m.js'; ...@@ -28,6 +28,7 @@ import {SyncBrowserProxyImpl} from '../people_page/sync_browser_proxy.m.js';
// <if expr="chromeos"> // <if expr="chromeos">
import {BlockingRequestManager} from './blocking_request_manager.js'; import {BlockingRequestManager} from './blocking_request_manager.js';
import {MultiStorePasswordUiEntry} from './multi_store_password_ui_entry.js';
// </if> // </if>
import {PasswordMoreActionsClickedEvent} from './password_list_item.js'; import {PasswordMoreActionsClickedEvent} from './password_list_item.js';
import {PasswordManagerImpl, PasswordManagerProxy} from './password_manager_proxy.js'; import {PasswordManagerImpl, PasswordManagerProxy} from './password_manager_proxy.js';
...@@ -39,6 +40,15 @@ Polymer({ ...@@ -39,6 +40,15 @@ Polymer({
_template: html`{__html_template__}`, _template: html`{__html_template__}`,
properties: { properties: {
/**
* Saved passwords after deduplicating versions that are repeated in the
* account and on the device.
* @type {!Array<!MultiStorePasswordUiEntry>}
*/
savedPasswords: {
type: Array,
value: () => [],
},
/** /**
* The model for any active menus or dialogs. The value is reset to null * The model for any active menus or dialogs. The value is reset to null
......
...@@ -165,7 +165,8 @@ ...@@ -165,7 +165,8 @@
<if expr="chromeos"> <if expr="chromeos">
token-request-manager="[[tokenRequestManager_]]" token-request-manager="[[tokenRequestManager_]]"
</if> </if>
should-show-storage-details="[[shouldShowStorageDetails_]]"> should-show-storage-details="[[shouldShowStorageDetails_]]"
saved-passwords="[[savedPasswords]]">
<div slot="body" class="list-frame"> <div slot="body" class="list-frame">
<div hidden$="[[!eligibleForAccountStorage_]]" <div hidden$="[[!eligibleForAccountStorage_]]"
id="accountStorageButtonsContainer"> id="accountStorageButtonsContainer">
......
...@@ -335,6 +335,27 @@ TEST_F(PasswordManagerPresenterTest, ...@@ -335,6 +335,27 @@ TEST_F(PasswordManagerPresenterTest,
ElementsAre(Pair(kNewUser, kNewPass))); ElementsAre(Pair(kNewUser, kNewPass)));
} }
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordBySortKey_ChangeUsernameAndPasswordForAllEntities) {
char kMobileExampleCom[] = "https://m.example.com/";
AddPasswordEntry(GURL(kExampleCom), kUsername, kPassword);
AddPasswordEntry(GURL(kMobileExampleCom), kUsername, kPassword);
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair(kUsername, kPassword)));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword, kNewUser,
kNewPass);
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair(kNewUser, kNewPass)));
EXPECT_THAT(
GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kMobileExampleCom)),
ElementsAre(Pair(kNewUser, kNewPass)));
}
TEST_F(PasswordManagerPresenterTest, TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordBySortKey_RejectSameUsernameForSameRealm) { ChangeSavedPasswordBySortKey_RejectSameUsernameForSameRealm) {
AddPasswordEntry(GURL(kExampleCom), kUsername, kPassword); AddPasswordEntry(GURL(kExampleCom), kUsername, kPassword);
......
...@@ -924,6 +924,7 @@ void AddAutofillStrings(content::WebUIDataSource* html_source, ...@@ -924,6 +924,7 @@ void AddAutofillStrings(content::WebUIDataSource* html_source,
{"editPasswordTitle", IDS_SETTINGS_PASSWORD_EDIT_TITLE}, {"editPasswordTitle", IDS_SETTINGS_PASSWORD_EDIT_TITLE},
{"editPassword", IDS_SETTINGS_PASSWORD_EDIT}, {"editPassword", IDS_SETTINGS_PASSWORD_EDIT},
{"editPasswordFootnote", IDS_SETTINGS_PASSWORD_EDIT_FOOTNOTE}, {"editPasswordFootnote", IDS_SETTINGS_PASSWORD_EDIT_FOOTNOTE},
{"usernameAlreadyUsed", IDS_SETTINGS_PASSWORD_USERNAME_ALREADY_USED},
{"copyPassword", IDS_SETTINGS_PASSWORD_COPY}, {"copyPassword", IDS_SETTINGS_PASSWORD_COPY},
{"passwordStoredOnDevice", IDS_SETTINGS_PASSWORD_STORED_ON_DEVICE}, {"passwordStoredOnDevice", IDS_SETTINGS_PASSWORD_STORED_ON_DEVICE},
{"passwordStoredInAccount", IDS_SETTINGS_PASSWORD_STORED_IN_ACCOUNT}, {"passwordStoredInAccount", IDS_SETTINGS_PASSWORD_STORED_IN_ACCOUNT},
......
...@@ -193,7 +193,7 @@ function detailsDialogPartsAreShownCorrectly(passwordDialog) { ...@@ -193,7 +193,7 @@ function detailsDialogPartsAreShownCorrectly(passwordDialog) {
async function changeSavedPasswordTestHelper( async function changeSavedPasswordTestHelper(
editDialog, entryIds, passwordManager) { editDialog, entryIds, passwordManager) {
const PASSWORD1 = 'hello_world'; const PASSWORD1 = 'hello_world';
const USERNAME1 = 'new_username';
editDialog.set('entry.password', PASSWORD1); editDialog.set('entry.password', PASSWORD1);
assertEquals(PASSWORD1, editDialog.$.passwordInput.value); assertEquals(PASSWORD1, editDialog.$.passwordInput.value);
...@@ -203,6 +203,7 @@ async function changeSavedPasswordTestHelper( ...@@ -203,6 +203,7 @@ async function changeSavedPasswordTestHelper(
assertTrue(editDialog.$.actionButton.disabled); assertTrue(editDialog.$.actionButton.disabled);
const PASSWORD2 = 'hello_world_2'; const PASSWORD2 = 'hello_world_2';
editDialog.$.usernameInput.value = USERNAME1;
editDialog.$.passwordInput.value = PASSWORD2; editDialog.$.passwordInput.value = PASSWORD2;
assertFalse(editDialog.$.passwordInput.invalid); assertFalse(editDialog.$.passwordInput.invalid);
assertFalse(editDialog.$.actionButton.disabled); assertFalse(editDialog.$.actionButton.disabled);
...@@ -210,8 +211,9 @@ async function changeSavedPasswordTestHelper( ...@@ -210,8 +211,9 @@ async function changeSavedPasswordTestHelper(
editDialog.$.actionButton.click(); editDialog.$.actionButton.click();
// Check that the changeSavedPassword is called with the right arguments. // Check that the changeSavedPassword is called with the right arguments.
const {ids, newPassword} = const {ids, newUsername, newPassword} =
await passwordManager.whenCalled('changeSavedPassword'); await passwordManager.whenCalled('changeSavedPassword');
assertEquals(USERNAME1, newUsername);
assertEquals(PASSWORD2, newPassword); assertEquals(PASSWORD2, newPassword);
assertEquals(entryIds.length, ids.length); assertEquals(entryIds.length, ids.length);
...@@ -1160,6 +1162,26 @@ suite('PasswordsSection', function() { ...@@ -1160,6 +1162,26 @@ suite('PasswordsSection', function() {
passwordManager); passwordManager);
}); });
test('editDialogChangeUsernameFailsWhenReused', async function() {
loadTimeData.overrideValues({editPasswordsInSettings: true});
const accountEntry = createMultiStorePasswordEntry(
{url: 'goo.gl', username: 'bart', accountId: 0});
const editDialog = elementFactory.createPasswordEditDialog(accountEntry);
editDialog.usernamesForSameOrigin = new Set(['mark', 'bart']);
editDialog.$.usernameInput.value = 'mark';
assertTrue(editDialog.$.usernameInput.invalid);
assertTrue(editDialog.$.actionButton.disabled);
editDialog.$.usernameInput.value = 'new_mark';
assertFalse(editDialog.$.usernameInput.invalid);
assertFalse(editDialog.$.actionButton.disabled);
changeSavedPasswordTestHelper(
editDialog, [accountEntry.accountId], passwordManager);
});
// Test verifies that the edit dialog informs the password is stored in the // Test verifies that the edit dialog informs the password is stored in the
// account. // account.
test('verifyStorageDetailsInEditDialogForAccountPassword', function() { test('verifyStorageDetailsInEditDialogForAccountPassword', 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