Commit 41c3d488 authored by btapiz's avatar btapiz Committed by Commit bot

Allow editing passwords in settings/passwords

BUG=377410
R=vabr@chromium.org,dbeam@chromium.org
TEST=In chrome://settings/passwords, revealed passwords should be editable.

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

Cr-Commit-Position: refs/heads/master@{#295912}
parent e548913a
......@@ -7270,12 +7270,30 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_PASSWORDS_PAGE_SEARCH_PASSWORDS" desc="Placeholder text shown in password table search box">
Search passwords
</message>
<message name="IDS_PASSWORDS_PAGE_URL_INSTRUCTION" desc="Placeholder text shown in the URL field when manually adding new credentials on the passwords page in settings.">
Login page URL
</message>
<message name="IDS_PASSWORDS_PAGE_USERNAME_INSTRUCTION" desc="Placeholder text shown in the username field when manually adding new credentials on the passwords page in settings.">
Username
</message>
<message name="IDS_PASSWORDS_PAGE_PASSWORD_INSTRUCTION" desc="Placeholder text shown in the password field when manually adding new credentials on the passwords page in settings.">
Password
</message>
<message name="IDS_PASSWORDS_PAGE_INVALID_URL_TOOLTIP" desc="Tooltip text for an invalid login page URL when manually adding new credentials on the passwords page in settings.">
Login page URL must be a valid HTTP or HTTPS URL
</message>
<message name="IDS_PASSWORDS_PAGE_INVALID_PASSWORD_TOOLTIP" desc="Tooltip text for an invalid password when manually adding new credentials on the passwords page in settings.">
Password must contain at least one character
</message>
<message name="IDS_PASSWORDS_PAGE_VIEW_SHOW_BUTTON" desc="Text for passwords page view's button to show a stored password">
Show
</message>
<message name="IDS_PASSWORDS_PAGE_VIEW_HIDE_BUTTON" desc="Text for passwords page view's button to hide a stored password">
Hide
</message>
<message name="IDS_PASSWORDS_PAGE_VIEW_OVERWRITE_BUTTON" desc="Text for passwords page view's button to overwrite a stored password">
Overwrite
</message>
<message name="IDS_PASSWORDS_PAGE_VIEW_NO_PASSWORDS_DESCRIPTION" desc="Text for passwords page view when there are no passwords to show.">
Your saved passwords will appear here.
</message>
......
......@@ -55,6 +55,13 @@ cr.define('options', function() {
*/
lastQuery_: null,
/**
* Whether a search query filter is applied to the current data model.
* @type {boolean}
* @private
*/
filterIsActive_: false,
/** @override */
initializePage: function() {
Page.prototype.initializePage.call(this);
......@@ -158,6 +165,7 @@ cr.define('options', function() {
* @param {!Array} entries The list of saved password data.
*/
setSavedPasswordsList_: function(entries) {
this.filterIsActive_ = !!this.lastQuery_;
if (this.lastQuery_) {
// Implement password searching here in javascript, rather than in C++.
// The number of saved passwords shouldn't be too big for us to handle.
......@@ -174,6 +182,9 @@ cr.define('options', function() {
return false;
};
entries = entries.filter(filter);
} else {
// Adds the Add New Entry row.
entries.push(null);
}
this.savedPasswordsList_.dataModel = new ArrayDataModel(entries);
this.updateListVisibility_(this.savedPasswordsList_);
......@@ -197,7 +208,7 @@ cr.define('options', function() {
*/
showPassword_: function(index, password) {
var model = this.savedPasswordsList_.dataModel;
if (this.lastQuery_) {
if (this.filterIsActive_) {
// When a filter is active, |index| does not represent the current
// index in the model, but each entry stores its original index, so
// we can find the item using a linear search.
......@@ -213,6 +224,53 @@ cr.define('options', function() {
var item = this.savedPasswordsList_.getListItemByIndex(index);
item.showPassword(password);
},
/**
* Forwards the validity of the origin to the Add New Entry row.
* @param {string} url The origin.
* @param {boolean} valid The validity of the origin for adding.
* @private
*/
originValidityCheckComplete_: function(url, valid) {
// There is no Add New Entry row when a filter is active.
if (this.filterIsActive_)
return;
// Since no filter is active, the Add New Entry row always exists and its
// item is the last one.
var model = this.savedPasswordsList_.dataModel;
assert(model.length > 0);
var addRowItem = this.savedPasswordsList_.getListItemByIndex(
model.length - 1);
addRowItem.originValidityCheckComplete(url, valid);
},
};
/**
* Requests the browser to check the validity of the origin being edited by
* the user in the Add New Entry row.
* @param {string} url The origin being edited.
*/
PasswordManager.checkOriginValidityForAdding = function(url) {
chrome.send('checkOriginValidityForAdding', [url]);
};
/**
* Adds a new password entry.
* @param {string} url The origin.
* @param {string} username The username value.
* @param {string} password The password value.
*/
PasswordManager.addPassword = function(url, username, password) {
chrome.send('addPassword', [url, username, password]);
};
/**
* Updates the password value of an entry.
* @param {number} rowIndex The row to update.
* @param {string} newPassword The new password value.
*/
PasswordManager.updatePassword = function(rowIndex, newPassword) {
chrome.send('updatePassword', [String(rowIndex), newPassword]);
};
/**
......@@ -239,7 +297,8 @@ cr.define('options', function() {
cr.makePublic(PasswordManager, [
'setSavedPasswordsList',
'setPasswordExceptionsList',
'showPassword'
'showPassword',
'originValidityCheckComplete'
]);
// Export
......
......@@ -2,22 +2,29 @@
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file. */
#saved-passwords-list .list-inline-buttons-container {
display: flex;
position: absolute;
right: 0;
top: 3px;
}
html[dir='rtl'] #saved-passwords-list .list-inline-buttons-container {
left: 0;
right: auto;
}
#saved-passwords-list .list-inline-button {
-webkit-margin-end: 2px;
-webkit-transition: opacity 150ms;
background: rgb(138, 170, 237);
font-size: 0.9em;
height: 18px;
padding: 0 2px;
position: absolute;
top: 3px;
}
html[dir='ltr'] #saved-passwords-list .list-inline-button {
right: 2px;
}
html[dir='rtl'] #saved-passwords-list .list-inline-button {
left: 2px;
#saved-passwords-list div[role=listitem]:not([selected]) .list-inline-button {
display: none;
}
input[type='password'].inactive-password {
......@@ -26,12 +33,14 @@ input[type='password'].inactive-password {
}
#saved-passwords-list .url {
-webkit-padding-end: 0.5em;
box-sizing: border-box;
width: 40%;
}
#saved-passwords-list .name {
-webkit-box-flex: 1;
-webkit-padding-end: 0.5em;
width: 20%;
}
......@@ -40,6 +49,8 @@ input[type='password'].inactive-password {
position: relative;
}
#saved-passwords-list .url input[type='text'],
#saved-passwords-list .name input[type='text'],
#saved-passwords-list .password input[type='password'],
#saved-passwords-list .password input[type='text'] {
box-sizing: border-box;
......
......@@ -40,6 +40,15 @@ PasswordManagerPresenter::~PasswordManagerPresenter() {
store->RemoveObserver(this);
}
// static
bool PasswordManagerPresenter::CheckOriginValidityForAdding(
const GURL& origin) {
// Restrict the URL scheme to http and https since a manually-added
// PasswordForm entry's |scheme| is assumed to be SCHEME_HTML.
return origin.is_valid() && (origin.SchemeIs(url::kHttpScheme) ||
origin.SchemeIs(url::kHttpsScheme));
}
void PasswordManagerPresenter::Initialize() {
// Due to the way that handlers are (re)initialized under certain types of
// navigation, the presenter may already be initialized. (See bugs 88986
......@@ -83,6 +92,68 @@ void PasswordManagerPresenter::UpdatePasswordLists() {
exception_populater_.Populate();
}
void PasswordManagerPresenter::AddPassword(
const GURL& origin,
const base::string16& username_value,
const base::string16& password_value) {
#if defined(OS_ANDROID)
NOTREACHED();
#else
if (!CheckOriginValidityForAdding(origin) || password_value.empty()) {
// Invalid |origin| or empty |password_value| can only come from a
// compromised renderer.
NOTREACHED();
return;
}
PasswordStore* store = GetPasswordStore();
if (!store)
return;
GURL::Replacements replacements;
replacements.ClearUsername();
replacements.ClearPassword();
replacements.ClearQuery();
replacements.ClearRef();
autofill::PasswordForm form;
form.origin = origin.ReplaceComponents(replacements);
form.username_value = username_value;
form.password_value = password_value;
form.signon_realm = origin.GetOrigin().spec();
form.date_created = base::Time::Now();
// Because a secure scheme does not imply the presence of a valid certificate,
// this is not precise. However we give it the benefit of the doubt so that
// PasswordForms with a https origin will not be auto-filled unless the form
// comes with a valid SSL certificate.
form.ssl_valid = origin.SchemeIsSecure();
store->AddLogin(form);
#endif
}
void PasswordManagerPresenter::UpdatePassword(
size_t index,
const base::string16& password_value) {
#if defined(OS_ANDROID)
NOTREACHED();
#else
if (index >= password_list_.size() || password_value.empty()) {
// |index| out of bounds might come from a compromised renderer, don't let
// it crash the browser. http://crbug.com/362054
// Similarly, empty |password_value| also might come from a compromised
// renderer. So use the same logic to prevent saving it.
NOTREACHED();
return;
}
PasswordStore* store = GetPasswordStore();
if (!store)
return;
autofill::PasswordForm form(*password_list_[index]);
form.password_value = password_value;
store->UpdateLogin(form);
#endif
}
void PasswordManagerPresenter::RemoveSavedPassword(size_t index) {
if (index >= password_list_.size()) {
// |index| out of bounds might come from a compromised renderer, don't let
......
......@@ -5,22 +5,24 @@
#ifndef CHROME_BROWSER_UI_PASSWORDS_PASSWORD_MANAGER_PRESENTER_H_
#define CHROME_BROWSER_UI_PASSWORDS_PASSWORD_MANAGER_PRESENTER_H_
#include <stddef.h>
#include <string>
#include <vector>
#include "base/memory/scoped_vector.h"
#include "base/prefs/pref_member.h"
#include "base/strings/string16.h"
#include "components/password_manager/core/browser/password_store.h"
#include "components/password_manager/core/browser/password_store_consumer.h"
class GURL;
class PasswordUIView;
class Profile;
namespace autofill {
struct PasswordForm;
}
class PasswordUIView;
class Profile;
// Contains the common logic used by a PasswordUIView to
// interact with PasswordStore. It provides completion callbacks for
// PasswordStore operations and updates the view on PasswordStore changes.
......@@ -31,6 +33,9 @@ class PasswordManagerPresenter
explicit PasswordManagerPresenter(PasswordUIView* password_view);
virtual ~PasswordManagerPresenter();
// Checks if |origin| is valid for adding a new password entry.
static bool CheckOriginValidityForAdding(const GURL& origin);
// PasswordStore::Observer implementation.
virtual void OnLoginsChanged(
const password_manager::PasswordStoreChangeList& changes) OVERRIDE;
......@@ -46,6 +51,17 @@ class PasswordManagerPresenter
// Gets the password exception entry at |index|.
const autofill::PasswordForm* GetPasswordException(size_t index);
// Adds a new password entry with |origin|, |username_value|, and
// |password_value|. |origin| should have been validated by
// CheckOriginValidityForAdding, and |password_value| should be non-empty.
void AddPassword(const GURL& origin,
const base::string16& username_value,
const base::string16& password_value);
// Updates the entry at |index| with |password_value|. |password_value| should
// be non-empty.
void UpdatePassword(size_t index, const base::string16& password_value);
// Removes the saved password entry at |index|.
// |index| the entry index to be removed.
void RemoveSavedPassword(size_t index);
......
......@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/macros.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/password_manager/mock_password_store_service.h"
#include "chrome/browser/password_manager/password_store_factory.h"
......@@ -14,6 +15,7 @@
using base::ASCIIToUTF16;
using testing::Eq;
using testing::Field;
using testing::Property;
class MockPasswordUIView : public PasswordUIView {
......@@ -57,6 +59,9 @@ class PasswordManagerPresenterTest : public testing::Test {
PasswordStoreFactory::GetInstance()->SetTestingFactory(
&profile_, MockPasswordStoreService::Build);
mock_controller_.reset(new MockPasswordUIView(&profile_));
mock_store_ = static_cast<password_manager::MockPasswordStore*>(
PasswordStoreFactory::GetForProfile(&profile_,
Profile::EXPLICIT_ACCESS).get());
}
void AddPasswordEntry(const GURL& origin,
const std::string& user_name,
......@@ -64,10 +69,17 @@ class PasswordManagerPresenterTest : public testing::Test {
void AddPasswordException(const GURL& origin);
void UpdateLists();
MockPasswordUIView* GetUIController() { return mock_controller_.get(); }
PasswordManagerPresenter* GetPasswordManagerPresenter() {
return mock_controller_->GetPasswordManagerPresenter();
}
password_manager::MockPasswordStore* GetPasswordStore() {
return mock_store_.get();
}
private:
TestingProfile profile_;
scoped_ptr<MockPasswordUIView> mock_controller_;
scoped_refptr<password_manager::MockPasswordStore> mock_store_;
DISALLOW_COPY_AND_ASSIGN(PasswordManagerPresenterTest);
};
......@@ -78,24 +90,22 @@ void PasswordManagerPresenterTest::AddPasswordEntry(
const std::string& password) {
autofill::PasswordForm* form = new autofill::PasswordForm();
form->origin = origin;
form->username_element = base::ASCIIToUTF16("Email");
form->username_value = base::ASCIIToUTF16(user_name);
form->password_element = base::ASCIIToUTF16("Passwd");
form->password_value = base::ASCIIToUTF16(password);
mock_controller_->GetPasswordManagerPresenter()->password_list_
.push_back(form);
form->username_element = ASCIIToUTF16("Email");
form->username_value = ASCIIToUTF16(user_name);
form->password_element = ASCIIToUTF16("Passwd");
form->password_value = ASCIIToUTF16(password);
GetPasswordManagerPresenter()->password_list_.push_back(form);
}
void PasswordManagerPresenterTest::AddPasswordException(const GURL& origin) {
autofill::PasswordForm* form = new autofill::PasswordForm();
form->origin = origin;
mock_controller_->GetPasswordManagerPresenter()->password_exception_list_
.push_back(form);
GetPasswordManagerPresenter()->password_exception_list_.push_back(form);
}
void PasswordManagerPresenterTest::UpdateLists() {
mock_controller_->GetPasswordManagerPresenter()->SetPasswordList();
mock_controller_->GetPasswordManagerPresenter()->SetPasswordExceptionList();
GetPasswordManagerPresenter()->SetPasswordList();
GetPasswordManagerPresenter()->SetPasswordExceptionList();
}
namespace {
......@@ -145,4 +155,106 @@ TEST_F(PasswordManagerPresenterTest, UIControllerIsCalled) {
UpdateLists();
}
// AddPassword and UpdatePassword are never called on Android.
#if !defined(OS_ANDROID)
TEST_F(PasswordManagerPresenterTest, CallAddPassword) {
GURL basic_origin("http://host.com");
base::string16 username = ASCIIToUTF16("username");
base::string16 password = ASCIIToUTF16("password");
EXPECT_CALL(
*GetPasswordStore(),
AddLogin(testing::AllOf(
Field(&autofill::PasswordForm::signon_realm, Eq(basic_origin.spec())),
Field(&autofill::PasswordForm::origin, Eq(basic_origin)),
Field(&autofill::PasswordForm::username_value, Eq(username)),
Field(&autofill::PasswordForm::password_value, Eq(password)),
Field(&autofill::PasswordForm::ssl_valid, Eq(false)))));
GetPasswordManagerPresenter()->AddPassword(basic_origin, username, password);
GURL complex_origin("https://foo:bar@host.com:1234/path?query=v#ref");
EXPECT_CALL(
*GetPasswordStore(),
AddLogin(testing::AllOf(
Field(&autofill::PasswordForm::signon_realm,
Eq("https://host.com:1234/")),
Field(&autofill::PasswordForm::origin,
Eq(GURL("https://host.com:1234/path"))),
Field(&autofill::PasswordForm::username_value, Eq(username)),
Field(&autofill::PasswordForm::password_value, Eq(password)),
Field(&autofill::PasswordForm::ssl_valid, Eq(true)))));
GetPasswordManagerPresenter()->AddPassword(complex_origin,
username,
password);
}
TEST_F(PasswordManagerPresenterTest, CallUpdatePassword) {
GURL origin1("http://host.com");
static const char kUsername1[] = "username";
AddPasswordEntry(origin1, kUsername1, "password");
GURL origin2("https://example.com");
static const char kUsername2[] = "testname";
AddPasswordEntry(origin2, kUsername2, "abcd");
base::string16 new_password = ASCIIToUTF16("testpassword");
EXPECT_CALL(
*GetPasswordStore(),
UpdateLogin(testing::AllOf(
Field(&autofill::PasswordForm::origin, Eq(origin1)),
Field(&autofill::PasswordForm::username_value,
Eq(ASCIIToUTF16(kUsername1))),
Field(&autofill::PasswordForm::password_value,
Eq(new_password)))));
GetPasswordManagerPresenter()->UpdatePassword(0, new_password);
base::string16 new_password_again = ASCIIToUTF16("testpassword_again");
EXPECT_CALL(
*GetPasswordStore(),
UpdateLogin(testing::AllOf(
Field(&autofill::PasswordForm::origin, Eq(origin1)),
Field(&autofill::PasswordForm::username_value,
Eq(ASCIIToUTF16(kUsername1))),
Field(&autofill::PasswordForm::password_value,
Eq(new_password_again)))));
GetPasswordManagerPresenter()->UpdatePassword(0, new_password_again);
base::string16 another_password = ASCIIToUTF16("mypassword");
EXPECT_CALL(
*GetPasswordStore(),
UpdateLogin(testing::AllOf(
Field(&autofill::PasswordForm::origin, Eq(origin2)),
Field(&autofill::PasswordForm::username_value,
Eq(ASCIIToUTF16(kUsername2))),
Field(&autofill::PasswordForm::password_value,
Eq(another_password)))));
GetPasswordManagerPresenter()->UpdatePassword(1, another_password);
}
#endif // !defined(OS_ANDROID)
TEST(PasswordManagerPresenterTestSimple, CallCheckOriginValidityForAdding) {
static const char* const kValidOrigins[] = {
"http://host.com",
"http://host.com/path",
"https://host.com",
"https://foo:bar@host.com/path?query=v#ref",
"https://foo:bar@host.com:1234/path?query=v#ref"
};
for (size_t i = 0; i < arraysize(kValidOrigins); ++i) {
SCOPED_TRACE(kValidOrigins[i]);
EXPECT_TRUE(PasswordManagerPresenter::CheckOriginValidityForAdding(
GURL(kValidOrigins[i])));
}
static const char* const kInvalidOrigins[] = {
"noscheme",
"invalidscheme:host.com",
"ftp://ftp.host.com",
"about:test"
};
for (size_t i = 0; i < arraysize(kInvalidOrigins); ++i) {
SCOPED_TRACE(kInvalidOrigins[i]);
EXPECT_FALSE(PasswordManagerPresenter::CheckOriginValidityForAdding(
GURL(kInvalidOrigins[i])));
}
}
} // namespace
......@@ -53,10 +53,22 @@ void PasswordManagerHandler::GetLocalizedValues(
IDS_PASSWORDS_EXCEPTIONS_TAB_TITLE },
{ "passwordSearchPlaceholder",
IDS_PASSWORDS_PAGE_SEARCH_PASSWORDS },
{ "newPasswordUrlFieldPlaceholder",
IDS_PASSWORDS_PAGE_URL_INSTRUCTION },
{ "newPasswordUsernameFieldPlaceholder",
IDS_PASSWORDS_PAGE_USERNAME_INSTRUCTION },
{ "newPasswordPasswordFieldPlaceholder",
IDS_PASSWORDS_PAGE_PASSWORD_INSTRUCTION },
{ "editPasswordInvalidUrlTooltip",
IDS_PASSWORDS_PAGE_INVALID_URL_TOOLTIP },
{ "editPasswordInvalidPasswordTooltip",
IDS_PASSWORDS_PAGE_INVALID_PASSWORD_TOOLTIP },
{ "passwordShowButton",
IDS_PASSWORDS_PAGE_VIEW_SHOW_BUTTON },
{ "passwordHideButton",
IDS_PASSWORDS_PAGE_VIEW_HIDE_BUTTON },
{ "passwordOverwriteButton",
IDS_PASSWORDS_PAGE_VIEW_OVERWRITE_BUTTON },
{ "passwordsNoPasswordsDescription",
IDS_PASSWORDS_PAGE_VIEW_NO_PASSWORDS_DESCRIPTION },
{ "passwordsNoExceptionsDescription",
......@@ -88,6 +100,18 @@ void PasswordManagerHandler::RegisterMessages() {
"updatePasswordLists",
base::Bind(&PasswordManagerHandler::HandleUpdatePasswordLists,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"checkOriginValidityForAdding",
base::Bind(&PasswordManagerHandler::HandleCheckOriginValidityForAdding,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"addPassword",
base::Bind(&PasswordManagerHandler::HandleAddPassword,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"updatePassword",
base::Bind(&PasswordManagerHandler::HandleUpdatePassword,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"removeSavedPassword",
base::Bind(&PasswordManagerHandler::HandleRemoveSavedPassword,
......@@ -106,6 +130,44 @@ void PasswordManagerHandler::InitializeHandler() {
password_manager_presenter_.Initialize();
}
void PasswordManagerHandler::HandleCheckOriginValidityForAdding(
const base::ListValue* args) {
std::string origin;
const bool success = args->GetString(0, &origin);
DCHECK(success); // Don't CHECK here since the renderer might be compromised.
web_ui()->CallJavascriptFunction(
"PasswordManager.originValidityCheckComplete",
base::StringValue(origin),
base::FundamentalValue(
PasswordManagerPresenter::CheckOriginValidityForAdding(
GURL(origin))));
}
void PasswordManagerHandler::HandleAddPassword(const base::ListValue* args) {
std::string origin;
base::string16 username_value;
base::string16 password_value;
if (!args->GetString(0, &origin) || !args->GetString(1, &username_value) ||
!args->GetString(2, &password_value)) {
NOTREACHED();
return;
}
password_manager_presenter_.AddPassword(GURL(origin), username_value,
password_value);
}
void PasswordManagerHandler::HandleUpdatePassword(const base::ListValue* args) {
int index;
base::string16 password_value;
if (!ExtractIntegerValue(args, &index) || index < 0 ||
!args->GetString(1, &password_value)) {
NOTREACHED();
return;
}
password_manager_presenter_.UpdatePassword(static_cast<size_t>(index),
password_value);
}
void PasswordManagerHandler::HandleRemoveSavedPassword(
const base::ListValue* args) {
std::string string_value = base::UTF16ToUTF8(ExtractStringValue(args));
......
......@@ -46,6 +46,17 @@ class PasswordManagerHandler : public OptionsPageUIHandler,
// Called when the JS PasswordManager object is initialized.
void HandleUpdatePasswordLists(const base::ListValue* args);
// Checks if |origin| is valid for adding a new entry. Called while the user
// is editing an origin.
void HandleCheckOriginValidityForAdding(const base::ListValue* args);
// Adds a new password entry with |origin|, |username_value|, and
// |password_value|.
void HandleAddPassword(const base::ListValue* args);
// Updates the entry at |index| with |password_value|.
void HandleUpdatePassword(const base::ListValue* args);
// Removes a saved password entry.
// |value| the entry index to be removed.
void HandleRemoveSavedPassword(const base::ListValue* args);
......
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