Commit d79af158 authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

Reland "[Passwords] Remove passwords from the correct store in ..."

This is a reland of d705fd61

Original change's description:
> [Passwords] Remove passwords from the correct store in ...
> 
> ... CompromisedCredentialsManager()
> 
> Bug: 1108422
> Change-Id: I66ea95309a1c3242fe8e32591ae986d68dbe5ee2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2372429
> Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
> Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#801348}

Bug: 1108422
Change-Id: I27710fb2db2356ac5cce088ea1a27e67ac13d58c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2375466Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801728}
parent 59cc52f1
...@@ -216,15 +216,13 @@ void CompromisedCredentialsManager::SaveCompromisedCredential( ...@@ -216,15 +216,13 @@ void CompromisedCredentialsManager::SaveCompromisedCredential(
if (saved_password.password_value == credential.password() && if (saved_password.password_value == credential.password() &&
CanonicalizeUsername(saved_password.username_value) == CanonicalizeUsername(saved_password.username_value) ==
canonicalized_username) { canonicalized_username) {
PasswordStore& store = saved_password.IsUsingAccountStore() GetStoreFor(saved_password)
? *account_store_ .AddCompromisedCredentials({
: *profile_store_; .signon_realm = saved_password.signon_realm,
store.AddCompromisedCredentials({ .username = saved_password.username_value,
.signon_realm = saved_password.signon_realm, .create_time = base::Time::Now(),
.username = saved_password.username_value, .compromise_type = CompromiseType::kLeaked,
.create_time = base::Time::Now(), });
.compromise_type = CompromiseType::kLeaked,
});
} }
} }
} }
...@@ -243,7 +241,7 @@ bool CompromisedCredentialsManager::UpdateCompromisedCredentials( ...@@ -243,7 +241,7 @@ bool CompromisedCredentialsManager::UpdateCompromisedCredentials(
return false; return false;
for (size_t i = 1; i < forms.size(); ++i) for (size_t i = 1; i < forms.size(); ++i)
profile_store_->RemoveLogin(forms[i]); GetStoreFor(forms[i]).RemoveLogin(forms[i]);
// Note: We Invoke EditPassword on the presenter rather than UpdateLogin() on // Note: We Invoke EditPassword on the presenter rather than UpdateLogin() on
// the store, so that observers of the presenter get notified of this event. // the store, so that observers of the presenter get notified of this event.
...@@ -260,7 +258,7 @@ bool CompromisedCredentialsManager::RemoveCompromisedCredential( ...@@ -260,7 +258,7 @@ bool CompromisedCredentialsManager::RemoveCompromisedCredential(
// credentials were deleted. // credentials were deleted.
const auto& saved_passwords = it->second.forms; const auto& saved_passwords = it->second.forms;
for (const autofill::PasswordForm& saved_password : saved_passwords) for (const autofill::PasswordForm& saved_password : saved_passwords)
profile_store_->RemoveLogin(saved_password); GetStoreFor(saved_password).RemoveLogin(saved_password);
return !saved_passwords.empty(); return !saved_passwords.empty();
} }
...@@ -313,4 +311,9 @@ void CompromisedCredentialsManager::UpdateCachedDataAndNotifyObservers( ...@@ -313,4 +311,9 @@ void CompromisedCredentialsManager::UpdateCachedDataAndNotifyObservers(
} }
} }
PasswordStore& CompromisedCredentialsManager::GetStoreFor(
const autofill::PasswordForm& form) {
return form.IsUsingAccountStore() ? *account_store_ : *profile_store_;
}
} // namespace password_manager } // namespace password_manager
...@@ -162,6 +162,10 @@ class CompromisedCredentialsManager ...@@ -162,6 +162,10 @@ class CompromisedCredentialsManager
void UpdateCachedDataAndNotifyObservers( void UpdateCachedDataAndNotifyObservers(
SavedPasswordsPresenter::SavedPasswordsView saved_passwords); SavedPasswordsPresenter::SavedPasswordsView saved_passwords);
// Returns the `profile_store_` or `account_store_` if `form` is stored in the
// profile store of the account store accordingly.
PasswordStore& GetStoreFor(const autofill::PasswordForm& form);
// A weak handle to the presenter used to join the list of compromised // A weak handle to the presenter used to join the list of compromised
// credentials with saved passwords. Needs to outlive this instance. // credentials with saved passwords. Needs to outlive this instance.
SavedPasswordsPresenter* presenter_ = nullptr; SavedPasswordsPresenter* presenter_ = nullptr;
......
...@@ -642,4 +642,31 @@ TEST_F(CompromisedCredentialsManagerWithTwoStoresTest, ...@@ -642,4 +642,31 @@ TEST_F(CompromisedCredentialsManagerWithTwoStoresTest,
EXPECT_EQ(1U, profile_store().compromised_credentials().size()); EXPECT_EQ(1U, profile_store().compromised_credentials().size());
EXPECT_EQ(2U, account_store().compromised_credentials().size()); EXPECT_EQ(2U, account_store().compromised_credentials().size());
} }
TEST_F(CompromisedCredentialsManagerWithTwoStoresTest,
RemoveCompromisedCredential) {
// Add `kUsername1`,`kPassword1` to both stores.
profile_store().AddLogin(
MakeSavedPassword(kExampleCom, kUsername1, kPassword1));
account_store().AddLogin(
MakeSavedPassword(kExampleCom, kUsername1, kPassword1));
// Mark `kUsername1` and `kPassword1` to be compromised in both stores.
profile_store().AddCompromisedCredentials(
MakeCompromised(kExampleCom, kUsername1));
account_store().AddCompromisedCredentials(
MakeCompromised(kExampleCom, kUsername1));
RunUntilIdle();
// Now remove the compromised credentials
EXPECT_TRUE(provider().RemoveCompromisedCredential(
CredentialView(kExampleCom, GURL(), base::ASCIIToUTF16(kUsername1),
base::ASCIIToUTF16(kPassword1))));
RunUntilIdle();
// It should have been removed from both stores.
EXPECT_TRUE(profile_store().stored_passwords().at(kExampleCom).empty());
EXPECT_TRUE(account_store().stored_passwords().at(kExampleCom).empty());
}
} // namespace password_manager } // namespace password_manager
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