Commit 9a658d73 authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[Passwords] Disallow saving empty passwords in Password Check

This change disallows saving empty passwords in the edit password dialog
within the Password Check. It does so by introducing checks to both
JavaScript and C++.

Fixed: 1107401
Change-Id: Ib84ec706743d837464aba977bc2c08b83e83e7a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2307256Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatardpapad <dpapad@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790305}
parent 4b628d14
......@@ -316,6 +316,12 @@ ResponseAction PasswordsPrivateChangeCompromisedCredentialFunction::Run() {
*args_);
EXTENSION_FUNCTION_VALIDATE(parameters);
if (parameters->new_password.empty()) {
return RespondNow(
Error("Could not change the compromised credential. The new password "
"can't be empty."));
}
if (!GetDelegate(browser_context())
->ChangeCompromisedCredential(parameters->credential,
parameters->new_password)) {
......
......@@ -213,6 +213,13 @@ IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest,
<< message_;
}
IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest,
ChangeCompromisedCredentialWithEmptyPasswordFails) {
EXPECT_TRUE(
RunPasswordsSubtest("changeCompromisedCredentialWithEmptyPasswordFails"))
<< message_;
}
IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest,
ChangeCompromisedCredentialFails) {
EXPECT_TRUE(RunPasswordsSubtest("changeCompromisedCredentialFails"))
......
......@@ -29,7 +29,8 @@
</cr-input>
<cr-input id="passwordInput" value="[[item.password]]"
class="password-input" label="$i18n{editPasswordPasswordLabel}"
type="[[getPasswordInputType_(visible)]]">
type="[[getPasswordInputType_(visible)]]" required auto-validate
invalid="{{inputInvalid_}}">
<cr-icon-button id="showPasswordButton"
class$="[[showPasswordIcon_(visible)]]"
slot="suffix" on-click="onShowPasswordButtonClick_"
......@@ -42,7 +43,8 @@
<cr-button id="cancel" class="cancel-button" on-click="onCancel_">
$i18n{cancel}
</cr-button>
<cr-button id="save" class="action-button" on-click="onSave_">
<cr-button id="save" class="action-button" on-click="onSave_"
disabled="[[inputInvalid_]]">
$i18n{save}
</cr-button>
</div>
......
......@@ -48,6 +48,12 @@ Polymer({
type: Boolean,
value: false,
},
/**
* Whether the input is invalid.
* @private
*/
inputInvalid_: Boolean,
},
/** @private {?PasswordManagerProxy} */
......@@ -84,7 +90,7 @@ Polymer({
this.passwordManager_
.changeCompromisedCredential(
assert(this.item), this.$.passwordInput.value)
.then(() => {
.finally(() => {
this.close();
});
},
......
......@@ -351,6 +351,27 @@ var availableTests = [
});
},
function changeCompromisedCredentialWithEmptyPasswordFails() {
chrome.passwordsPrivate.changeCompromisedCredential(
{
id: 0,
formattedOrigin: 'example.com',
detailedOrigin: 'https://example.com',
isAndroidCredential: false,
signonRealm: 'https://example.com',
username: 'alice',
compromiseTime: COMPROMISE_TIME,
elapsedTimeSinceCompromise: '3 days ago',
compromiseType: 'LEAKED',
},
'', () => {
chrome.test.assertLastError(
'Could not change the compromised credential. The new password ' +
'can\'t be empty.');
chrome.test.succeed();
});
},
function changeCompromisedCredentialFails() {
chrome.passwordsPrivate.changeCompromisedCredential(
{
......
......@@ -1039,7 +1039,16 @@ suite('PasswordsCheckSection', function() {
const editDialog = createEditDialog(leakedPassword);
assertEquals(leakedPassword.password, editDialog.$.passwordInput.value);
// Test that an empty password is considered invalid and disables the change
// button.
editDialog.$.passwordInput.value = '';
assertTrue(editDialog.$.passwordInput.invalid);
assertTrue(editDialog.$.save.disabled);
editDialog.$.passwordInput.value = 'yadhtribym';
assertFalse(editDialog.$.passwordInput.invalid);
assertFalse(editDialog.$.save.disabled);
editDialog.$.save.click();
const interaction =
......
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