Skip to content

Commit

Permalink
Revert "[Passwords] Persist changes to insecure credentials in AddLogin"
Browse files Browse the repository at this point in the history
This reverts commit 9bff1a1.

Reason for revert: earl grey tests on iOS bots are breaking because of the update login change, however this has to be reverted first.

Bug: 1226812
Original change's description:
> [Passwords] Persist changes to insecure credentials in AddLogin
>
> If a password form contains insecure credentials when is being added to
> the db, they should be persisted too, without the need to call
> UpdateLogin after.
>
> Bug: 1223022
> Change-Id: I862b83fb0d6372b6e92493f23b4271a0f0c7c1b9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3001549
> Commit-Queue: Ioana Pandele <ioanap@chromium.org>
> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#898825}

Bug: 1223022
Change-Id: I3a36a547f48264a3d5acf7bab0a66de70333ad62
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3010094
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Maria Kazinova <kazinova@google.com>
Reviewed-by: Maria Kazinova <kazinova@google.com>
Owners-Override: Maria Kazinova <kazinova@google.com>
Auto-Submit: Ioana Pandele <ioanap@chromium.org>
Cr-Commit-Position: refs/heads/master@{#898869}
  • Loading branch information
Ioana Pandele authored and Chromium LUCI CQ committed Jul 6, 2021
1 parent fa74bda commit ea843d4
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 41 deletions.
22 changes: 5 additions & 17 deletions components/password_manager/core/browser/login_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1167,13 +1167,9 @@ PasswordStoreChangeList LoginDatabase::AddLogin(const PasswordForm& form,
if (success) {
// If success, the row never existed so password was not changed.
FillFormInStore(&form_with_encrypted_password);
FormPrimaryKey primary_key = FormPrimaryKey(db_.GetLastInsertRowId());
if (form_with_encrypted_password.password_issues.has_value()) {
UpdateInsecureCredentials(
primary_key, form_with_encrypted_password.password_issues.value());
}
list.emplace_back(PasswordStoreChange::ADD,
std::move(form_with_encrypted_password), primary_key,
std::move(form_with_encrypted_password),
FormPrimaryKey(db_.GetLastInsertRowId()),
/*password_changed=*/false);
return list;
}
Expand All @@ -1194,17 +1190,9 @@ PasswordStoreChangeList LoginDatabase::AddLogin(const PasswordForm& form,
list.emplace_back(PasswordStoreChange::REMOVE, removed_form,
FormPrimaryKey(old_primary_key_password.primary_key));
FillFormInStore(&form_with_encrypted_password);

FormPrimaryKey primary_key = FormPrimaryKey(db_.GetLastInsertRowId());
InsecureCredentialsChanged insecure_changed(false);
if (form_with_encrypted_password.password_issues.has_value()) {
insecure_changed = UpdateInsecureCredentials(
primary_key, form_with_encrypted_password.password_issues.value());
}
list.emplace_back(PasswordStoreChange::ADD,
std::move(form_with_encrypted_password),
FormPrimaryKey(db_.GetLastInsertRowId()),
password_changed, insecure_changed);
list.emplace_back(
PasswordStoreChange::ADD, std::move(form_with_encrypted_password),
FormPrimaryKey(db_.GetLastInsertRowId()), password_changed);
} else if (error) {
if (sqlite_error_code == 19 /*SQLITE_CONSTRAINT*/) {
*error = AddLoginError::kConstraintViolation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2888,30 +2888,6 @@ TEST_F(LoginDatabaseTest,
EXPECT_THAT(db().insecure_credentials_table().GetAllRows(), IsEmpty());
}

TEST_F(LoginDatabaseTest, AddLoginWithInsecureCredentialsPersistsThem) {
PasswordForm form = GenerateExamplePasswordForm();
InsecureCredential leaked{form.signon_realm, form.username_value,
base::Time(), InsecureType::kLeaked,
IsMuted(false)};
InsecureCredential phished = leaked;
phished.insecure_type = InsecureType::kPhished;

form.password_value = u"new_password";
form.password_issues = base::flat_map<InsecureType, InsecurityMetadata>();
form.password_issues->insert_or_assign(
InsecureType::kLeaked,
InsecurityMetadata(leaked.create_time, leaked.is_muted));
form.password_issues->insert_or_assign(
InsecureType::kPhished,
InsecurityMetadata(phished.create_time, phished.is_muted));

PasswordStoreChangeList list;
list.push_back(PasswordStoreChange(PasswordStoreChange::ADD, form));
EXPECT_EQ(list, db().AddLogin(form));
EXPECT_THAT(db().insecure_credentials_table().GetAllRows(),
testing::UnorderedElementsAre(leaked, phished));
}

class LoginDatabaseForAccountStoreTest : public testing::Test {
protected:
void SetUp() override {
Expand Down

0 comments on commit ea843d4

Please sign in to comment.