Commit 875621f9 authored by Ivana Zuzic's avatar Ivana Zuzic Committed by Commit Bot

[PWD Editing Android] Add ChangeSavedPassword function with a new signature

A new signature for ChangeSavedPassword takes the index of a password
entry, instead of its sort_key, because Android finds passwords by
index, rather than by sort_key.

Bug: 377410
Change-Id: I972091c7f59c6a0d3c9cc4cbbabea276fcba5c31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1731829Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Ivana Zuzic <izuzic@google.com>
Cr-Commit-Position: refs/heads/master@{#683996}
parent f0e581f7
......@@ -217,53 +217,25 @@ const autofill::PasswordForm* PasswordManagerPresenter::GetPasswordException(
}
void PasswordManagerPresenter::ChangeSavedPassword(
const std::string& sort_key,
base::string16 new_username,
base::Optional<base::string16> new_password) {
// If a password was provided, make sure it is not empty.
if (new_password && new_password->empty())
return;
size_t index,
const base::string16& new_username,
const base::Optional<base::string16>& new_password) {
DCHECK_LT(index, password_map_.size());
ChangeSavedPasswords(std::next(password_map_.begin(), index)->second,
new_username, new_password);
}
void PasswordManagerPresenter::ChangeSavedPassword(
const std::string& sort_key,
const base::string16& new_username,
const base::Optional<base::string16>& new_password) {
// Find the equivalence class that needs to be updated.
auto it = password_map_.find(sort_key);
if (it == password_map_.end())
return;
const auto& old_forms = it->second;
DCHECK(!old_forms.empty());
const std::string& signon_realm = old_forms[0]->signon_realm;
const base::string16& old_username = old_forms[0]->username_value;
const bool username_changed = old_username != new_username;
// In case the username changed, make sure that there exists no other
// credential with the same signon_realm and username.
if (username_changed) {
for (const auto& sort_key_passwords_pair : password_map_) {
for (const auto& password : sort_key_passwords_pair.second) {
if (password->signon_realm == signon_realm &&
password->username_value == new_username) {
return;
}
}
}
}
PasswordStore* store = GetPasswordStore();
if (!store)
return;
// An updated username implies a change in the primary key, thus we need to
// make sure to call the right API. Update every entry in the equivalence
// class.
for (const auto& old_form : old_forms) {
autofill::PasswordForm new_form = *old_form;
new_form.username_value = new_username;
if (new_password)
new_form.password_value = *new_password;
username_changed ? store->UpdateLoginWithPrimaryKey(new_form, *old_form)
: store->UpdateLogin(new_form);
}
ChangeSavedPasswords(it->second, new_username, new_password);
}
void PasswordManagerPresenter::RemoveSavedPassword(size_t index) {
......@@ -354,6 +326,54 @@ void PasswordManagerPresenter::RemoveLogin(const autofill::PasswordForm& form) {
store->RemoveLogin(form);
}
void PasswordManagerPresenter::ChangeSavedPasswords(
const FormVector& old_forms,
const base::string16& new_username,
const base::Optional<base::string16>& new_password) {
// If a password was provided, make sure it is not empty.
if (new_password && new_password->empty()) {
DLOG(ERROR) << "The password is empty.";
return;
}
DCHECK(!old_forms.empty());
const std::string& signon_realm = old_forms[0]->signon_realm;
const base::string16& old_username = old_forms[0]->username_value;
const bool username_changed = old_username != new_username;
// In case the username changed, make sure that there exists no other
// credential with the same signon_realm and username.
if (username_changed) {
for (const auto& sort_key_passwords_pair : password_map_) {
for (const auto& password : sort_key_passwords_pair.second) {
if (password->signon_realm == signon_realm &&
password->username_value == new_username) {
DLOG(ERROR) << "A credential with the same signon_realm and username "
"already exists.";
return;
}
}
}
}
PasswordStore* store = GetPasswordStore();
if (!store)
return;
// An updated username implies a change in the primary key, thus we need to
// make sure to call the right API. Update every entry in the equivalence
// class.
for (const auto& old_form : old_forms) {
autofill::PasswordForm new_form = *old_form;
new_form.username_value = new_username;
if (new_password)
new_form.password_value = *new_password;
username_changed ? store->UpdateLoginWithPrimaryKey(new_form, *old_form)
: store->UpdateLogin(new_form);
}
}
bool PasswordManagerPresenter::TryRemovePasswordEntries(
PasswordFormMap* form_map,
size_t index) {
......
......@@ -60,10 +60,14 @@ class PasswordManagerPresenter
// Gets the password exception entry at |index|.
const autofill::PasswordForm* GetPasswordException(size_t index) const;
// Changes the username and password corresponding to |sort_key|.
// Changes the username and password at |index|, or corresponding to
// |sort_key|.
void ChangeSavedPassword(size_t index,
const base::string16& new_username,
const base::Optional<base::string16>& new_password);
void ChangeSavedPassword(const std::string& sort_key,
base::string16 new_username,
base::Optional<base::string16> new_password);
const base::string16& new_username,
const base::Optional<base::string16>& new_password);
// Removes the saved password entries at |index|, or corresponding to
// |sort_key|, respectively.
......@@ -110,6 +114,12 @@ class PasswordManagerPresenter
std::map<std::string,
std::vector<std::unique_ptr<autofill::PasswordForm>>>;
// Implementation used in both |ChangeSavedPassword()| methods.
void ChangeSavedPasswords(
const std::vector<std::unique_ptr<autofill::PasswordForm>>& old_forms,
const base::string16& new_username,
const base::Optional<base::string16>& new_password);
// Attempts to remove the entries corresponding to |index| from |form_map|.
// This will also add a corresponding undo operation to |undo_manager_|.
// Returns whether removing the entry succeeded.
......
......@@ -31,6 +31,7 @@ using testing::_;
using testing::Each;
using testing::ElementsAre;
using testing::IsEmpty;
using testing::Pair;
using testing::SizeIs;
namespace {
......@@ -94,11 +95,12 @@ class PasswordManagerPresenterTest : public testing::Test {
store_->AddLogin(form);
}
void ChangeSavedPassword(base::StringPiece origin,
base::StringPiece old_username,
base::StringPiece old_password,
base::StringPiece new_username,
base::Optional<base::StringPiece> new_password) {
void ChangeSavedPasswordBySortKey(
base::StringPiece origin,
base::StringPiece old_username,
base::StringPiece old_password,
base::StringPiece new_username,
base::Optional<base::StringPiece> new_password) {
autofill::PasswordForm temp_form;
temp_form.origin = GURL(origin);
temp_form.signon_realm = temp_form.origin.GetOrigin().spec();
......@@ -117,6 +119,40 @@ class PasswordManagerPresenterTest : public testing::Test {
thread_bundle_.RunUntilIdle();
}
int GetPasswordIndex(base::StringPiece origin,
base::StringPiece old_username,
base::StringPiece old_password) {
std::vector<std::unique_ptr<autofill::PasswordForm>> password_forms =
mock_controller_.GetPasswordManagerPresenter()->GetAllPasswords();
return std::find_if(
password_forms.begin(), password_forms.end(),
[origin, old_username, old_password](const auto& password_form) {
return password_form->signon_realm ==
GURL(origin).GetOrigin().spec() &&
password_form->username_value ==
base::ASCIIToUTF16(old_username) &&
password_form->password_value ==
base::ASCIIToUTF16(old_password);
}) -
password_forms.begin();
}
void ChangeSavedPasswordByIndex(
base::StringPiece origin,
base::StringPiece old_username,
base::StringPiece old_password,
base::StringPiece new_username,
base::Optional<base::StringPiece> new_password) {
mock_controller_.GetPasswordManagerPresenter()->ChangeSavedPassword(
GetPasswordIndex(origin, old_username, old_password),
base::ASCIIToUTF16(new_username),
new_password ? base::make_optional(base::ASCIIToUTF16(*new_password))
: base::nullopt);
// The password store posts mutation tasks to a background thread, thus we
// need to spin the message loop here.
thread_bundle_.RunUntilIdle();
}
void UpdatePasswordLists() {
mock_controller_.GetPasswordManagerPresenter()->UpdatePasswordLists();
thread_bundle_.RunUntilIdle();
......@@ -145,96 +181,211 @@ class PasswordManagerPresenterTest : public testing::Test {
namespace {
TEST_F(PasswordManagerPresenterTest, ChangeSavedPassword_RejectEmptyPassword) {
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordBySortKey_RejectEmptyPassword) {
AddPasswordEntry(GURL(kExampleCom), "user", "pass");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("user", "pass")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPasswordBySortKey(kExampleCom, "user", "pass", "new_user", "");
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("user", "pass")));
}
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordByIndex_RejectEmptyPassword) {
AddPasswordEntry(GURL(kExampleCom), "user", "pass");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("user", "pass")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPasswordByIndex(kExampleCom, "user", "pass", "new_user", "");
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("user", "pass")));
}
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordBySortKey_ChangeUsername) {
AddPasswordEntry(GURL(kExampleCom), "user", "pass");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("user", "pass")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPasswordBySortKey(kExampleCom, "user", "pass", "new_user",
base::nullopt);
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("new_user", "pass")));
}
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordByIndex_ChangeUsername) {
AddPasswordEntry(GURL(kExampleCom), "user", "pass");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("user", "pass")));
ElementsAre(Pair("user", "pass")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPassword(kExampleCom, "user", "pass", "new_user", "");
ChangeSavedPasswordByIndex(kExampleCom, "user", "pass", "new_user",
base::nullopt);
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("user", "pass")));
ElementsAre(Pair("new_user", "pass")));
}
TEST_F(PasswordManagerPresenterTest, ChangeSavedPassword_ChangeUsername) {
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordBySortKey_ChangeUsernameAndPassword) {
AddPasswordEntry(GURL(kExampleCom), "user", "pass");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("user", "pass")));
ElementsAre(Pair("user", "pass")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPassword(kExampleCom, "user", "pass", "new_user", base::nullopt);
ChangeSavedPasswordBySortKey(kExampleCom, "user", "pass", "new_user",
"new_pass");
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("new_user", "pass")));
ElementsAre(Pair("new_user", "new_pass")));
}
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPassword_ChangeUsernameAndPassword) {
ChangeSavedPasswordByIndex_ChangeUsernameAndPassword) {
AddPasswordEntry(GURL(kExampleCom), "user", "pass");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("user", "pass")));
ElementsAre(Pair("user", "pass")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPassword(kExampleCom, "user", "pass", "new_user", "new_pass");
ChangeSavedPasswordByIndex(kExampleCom, "user", "pass", "new_user",
"new_pass");
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("new_user", "new_pass")));
ElementsAre(Pair("new_user", "new_pass")));
}
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPassword_RejectSameUsernameForSameRealm) {
ChangeSavedPasswordBySortKey_RejectSameUsernameForSameRealm) {
AddPasswordEntry(GURL(kExampleCom), "user", "pass");
AddPasswordEntry(GURL(kExampleCom), "user2", "pass2");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(2)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("user", "pass"),
std::make_pair("user2", "pass2")));
ElementsAre(Pair("user", "pass"), Pair("user2", "pass2")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPasswordBySortKey(kExampleCom, "user", "pass", "user2",
base::nullopt);
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("user", "pass"), Pair("user2", "pass2")));
}
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordByIndex_RejectSameUsernameForSameRealm) {
AddPasswordEntry(GURL(kExampleCom), "user", "pass");
AddPasswordEntry(GURL(kExampleCom), "user2", "pass2");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(2)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("user", "pass"), Pair("user2", "pass2")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPasswordByIndex(kExampleCom, "user", "pass", "user2",
base::nullopt);
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("user", "pass"), Pair("user2", "pass2")));
}
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordBySortKey_DontRejectSameUsernameForDifferentRealm) {
AddPasswordEntry(GURL(kExampleCom), "user", "pass");
AddPasswordEntry(GURL(kExampleOrg), "user2", "pass2");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(2)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("user", "pass")));
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleOrg)),
ElementsAre(Pair("user2", "pass2")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPassword(kExampleCom, "user", "pass", "user2", base::nullopt);
ChangeSavedPasswordBySortKey(kExampleCom, "user", "pass", "user2",
base::nullopt);
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("user", "pass"),
std::make_pair("user2", "pass2")));
ElementsAre(Pair("user2", "pass")));
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleOrg)),
ElementsAre(Pair("user2", "pass2")));
}
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPassword_DontRejectSameUsernameForDifferentRealm) {
ChangeSavedPasswordByIndex_DontRejectSameUsernameForDifferentRealm) {
AddPasswordEntry(GURL(kExampleCom), "user", "pass");
AddPasswordEntry(GURL(kExampleOrg), "user2", "pass2");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(2)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("user", "pass")));
ElementsAre(Pair("user", "pass")));
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleOrg)),
ElementsAre(std::make_pair("user2", "pass2")));
ElementsAre(Pair("user2", "pass2")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPassword(kExampleCom, "user", "pass", "user2", base::nullopt);
ChangeSavedPasswordByIndex(kExampleCom, "user", "pass", "user2",
base::nullopt);
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("user2", "pass")));
ElementsAre(Pair("user2", "pass")));
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleOrg)),
ElementsAre(std::make_pair("user2", "pass2")));
ElementsAre(Pair("user2", "pass2")));
}
TEST_F(PasswordManagerPresenterTest, ChangeSavedPassword_UpdateDuplicates) {
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordBySortKey_UpdateDuplicates) {
AddPasswordEntry(GURL(std::string(kExampleCom) + "pathA"), "user", "pass");
AddPasswordEntry(GURL(std::string(kExampleCom) + "pathB"), "user", "pass");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("user", "pass"),
std::make_pair("user", "pass")));
ElementsAre(Pair("user", "pass"), Pair("user", "pass")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPasswordBySortKey(kExampleCom, "user", "pass", "new_user",
"new_pass");
EXPECT_THAT(
GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("new_user", "new_pass"), Pair("new_user", "new_pass")));
}
ChangeSavedPassword(kExampleCom, "user", "pass", "new_user", "new_pass");
TEST_F(PasswordManagerPresenterTest,
ChangeSavedPasswordByIndex_UpdateDuplicates) {
AddPasswordEntry(GURL(std::string(kExampleCom) + "pathA"), "user", "pass");
AddPasswordEntry(GURL(std::string(kExampleCom) + "pathB"), "user", "pass");
EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
UpdatePasswordLists();
EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(std::make_pair("new_user", "new_pass"),
std::make_pair("new_user", "new_pass")));
ElementsAre(Pair("user", "pass"), Pair("user", "pass")));
testing::Mock::VerifyAndClearExpectations(&GetUIController());
ChangeSavedPasswordByIndex(kExampleCom, "user", "pass", "new_user",
"new_pass");
EXPECT_THAT(
GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
ElementsAre(Pair("new_user", "new_pass"), Pair("new_user", "new_pass")));
}
TEST_F(PasswordManagerPresenterTest, UIControllerIsCalled) {
......
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