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

[Passwords] Introduce FormFetcher.IsMovingBlocked() method

This method decides for a given user and username, whether moving
to the account store should be offerred or not.

Bug: 1032992
Change-Id: Ie57a2d4f4bafe309ac114f2060270ca3d85c0a6a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135612
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759049}
parent 5cc4cc8d
......@@ -52,6 +52,11 @@ bool FakeFormFetcher::IsBlacklisted() const {
return is_blacklisted_;
}
bool FakeFormFetcher::IsMovingBlocked(const autofill::GaiaIdHash& destination,
const base::string16& username) const {
return false;
}
const std::vector<const PasswordForm*>& FakeFormFetcher::GetAllRelevantMatches()
const {
return non_federated_same_scheme_;
......
......@@ -61,6 +61,8 @@ class FakeFormFetcher : public FormFetcher {
const override;
bool IsBlacklisted() const override;
bool IsMovingBlocked(const autofill::GaiaIdHash& destination,
const base::string16& username) const override;
const std::vector<const autofill::PasswordForm*>& GetAllRelevantMatches()
const override;
......
......@@ -11,6 +11,7 @@
#include "base/macros.h"
#include "base/observer_list_types.h"
#include "base/strings/string16.h"
#include "components/autofill/core/common/gaia_id_hash.h"
namespace autofill {
struct PasswordForm;
......@@ -79,6 +80,13 @@ class FormFetcher {
// GetState() returns NOT_WAITING.
virtual bool IsBlacklisted() const = 0;
// Whether moving the credentials with |username| from the
// local store to the account store for the user with
// |destination| GaiaIdHash is blocked. This is relevant only for account
// store users.
virtual bool IsMovingBlocked(const autofill::GaiaIdHash& destination,
const base::string16& username) const = 0;
// Non-federated matches obtained from the backend that have the same scheme
// of this form.
virtual const std::vector<const autofill::PasswordForm*>&
......
......@@ -146,6 +146,12 @@ bool FormFetcherImpl::IsBlacklisted() const {
return is_blacklisted_;
}
bool FormFetcherImpl::IsMovingBlocked(const autofill::GaiaIdHash& destination,
const base::string16& username) const {
NOTREACHED();
return false;
}
const std::vector<const PasswordForm*>& FormFetcherImpl::GetAllRelevantMatches()
const {
return non_federated_same_scheme_;
......
......@@ -53,6 +53,9 @@ class FormFetcherImpl : public FormFetcher,
std::vector<const autofill::PasswordForm*> GetFederatedMatches()
const override;
bool IsBlacklisted() const override;
bool IsMovingBlocked(const autofill::GaiaIdHash& destination,
const base::string16& username) const override;
const std::vector<const autofill::PasswordForm*>& GetAllRelevantMatches()
const override;
const std::vector<const autofill::PasswordForm*>& GetBestMatches()
......@@ -70,6 +73,14 @@ class FormFetcherImpl : public FormFetcher,
std::vector<std::unique_ptr<autofill::PasswordForm>> forms) override;
protected:
// Processes password form results and forwards them to the |consumers_|.
void ProcessPasswordStoreResults(
std::vector<std::unique_ptr<autofill::PasswordForm>> results);
// Splits |results| into |federated_|, |non_federated_| and |blacklisted_|.
virtual void SplitResults(
std::vector<std::unique_ptr<autofill::PasswordForm>> results);
// PasswordStore results will be fetched for this description.
const PasswordStore::FormDigest form_digest_;
......@@ -83,15 +94,6 @@ class FormFetcherImpl : public FormFetcher,
// password store returning results in the meantime.
bool need_to_refetch_ = false;
// Processes password form results and forwards them to the |consumers_|.
void ProcessPasswordStoreResults(
std::vector<std::unique_ptr<autofill::PasswordForm>> results);
// Splits |results| into |federated_|, |non_federated_| and |blacklisted_|.
virtual void SplitResults(
std::vector<std::unique_ptr<autofill::PasswordForm>> results);
private:
// Results obtained from PasswordStore:
std::vector<std::unique_ptr<autofill::PasswordForm>> non_federated_;
......@@ -100,6 +102,7 @@ class FormFetcherImpl : public FormFetcher,
// non-federated matches.
std::vector<std::unique_ptr<autofill::PasswordForm>> federated_;
private:
// Non-federated credentials of the same scheme as the observed form.
std::vector<const autofill::PasswordForm*> non_federated_same_scheme_;
......
......@@ -63,6 +63,29 @@ bool MultiStoreFormFetcher::IsBlacklisted() const {
return is_blacklisted_in_profile_store_;
}
bool MultiStoreFormFetcher::IsMovingBlocked(
const autofill::GaiaIdHash& destination,
const base::string16& username) const {
for (const std::vector<std::unique_ptr<autofill::PasswordForm>>*
matches_vector : {&federated_, &non_federated_}) {
for (const auto& form : *matches_vector) {
// Only local entries can be moved to the account store (though
// account store matches should never have |moving_blocked_for_list|
// entries anyway).
if (form->IsUsingAccountStore())
continue;
// Ignore PSL matches for blocking moving.
if (form->is_public_suffix_match)
continue;
if (form->username_value != username)
continue;
if (base::Contains(form->moving_blocked_for_list, destination))
return true;
}
}
return false;
}
void MultiStoreFormFetcher::OnGetPasswordStoreResults(
std::vector<std::unique_ptr<PasswordForm>> results) {
DCHECK_EQ(State::WAITING, state_);
......
......@@ -24,6 +24,8 @@ class MultiStoreFormFetcher : public FormFetcherImpl {
// FormFetcher overrides.
void Fetch() override;
bool IsBlacklisted() const override;
bool IsMovingBlocked(const autofill::GaiaIdHash& destination,
const base::string16& username) const override;
void OnGetPasswordStoreResults(
std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
......
......@@ -7,6 +7,7 @@
#include "base/strings/utf_string_conversions.h"
#include "base/test/task_environment.h"
#include "build/build_config.h"
#include "components/autofill/core/common/gaia_id_hash.h"
#include "components/autofill/core/common/password_form.h"
#include "components/password_manager/core/browser/mock_password_store.h"
#include "components/password_manager/core/browser/stub_password_manager_client.h"
......@@ -14,6 +15,7 @@
#include "url/origin.h"
#include "url/url_constants.h"
using autofill::GaiaIdHash;
using autofill::PasswordForm;
using base::ASCIIToUTF16;
using testing::_;
......@@ -65,7 +67,7 @@ class FakePasswordManagerClient : public StubPasswordManagerClient {
PasswordForm CreateHTMLForm(const std::string& origin_url,
const std::string& username_value,
const std::string& password_value,
base::Time date_last_used) {
base::Time date_last_used = base::Time::Now()) {
PasswordForm form;
form.scheme = PasswordForm::Scheme::kHtml;
form.origin = GURL(origin_url);
......@@ -294,4 +296,49 @@ TEST_F(MultiStoreFormFetcherTest, BlacklistEntryInTheProfileStore) {
EXPECT_TRUE(form_fetcher_->IsBlacklisted());
}
TEST_F(MultiStoreFormFetcherTest, MovingToAccountStoreIsBlocked) {
Fetch();
const GaiaIdHash kUser = GaiaIdHash::FromGaiaId("user");
const GaiaIdHash kAnotherUser = GaiaIdHash::FromGaiaId("another_user");
// Form that's blocked for |kUser| for "username1".
PasswordForm blocked_form =
CreateHTMLForm("www.url.com", "username1", "pass");
blocked_form.in_store = PasswordForm::Store::kProfileStore;
blocked_form.moving_blocked_for_list.push_back(kUser);
// Form that not blocked for "username2".
PasswordForm unblocked_form =
CreateHTMLForm("www.url.com", "username2", "pass");
unblocked_form.in_store = PasswordForm::Store::kProfileStore;
// PSL form that's blocked for |kUser| for "psl_username".
PasswordForm psl_form = CreateHTMLForm("psl.url.com", "psl_username", "pass");
psl_form.is_public_suffix_match = true;
psl_form.in_store = PasswordForm::Store::kProfileStore;
psl_form.moving_blocked_for_list.push_back(kUser);
// Pass response from the local store.
std::vector<std::unique_ptr<PasswordForm>> results;
results.push_back(std::make_unique<PasswordForm>(blocked_form));
results.push_back(std::make_unique<PasswordForm>(unblocked_form));
results.push_back(std::make_unique<PasswordForm>(psl_form));
form_fetcher_->OnGetPasswordStoreResults(std::move(results));
// Pass empty response from the account store.
form_fetcher_->OnGetPasswordStoreResults({});
// Moving should be blocked for |kUser| and |form1|.
EXPECT_TRUE(
form_fetcher_->IsMovingBlocked(kUser, blocked_form.username_value));
// Moving shouldn't be blocked for other usernames.
EXPECT_FALSE(
form_fetcher_->IsMovingBlocked(kUser, unblocked_form.username_value));
// Moving shouldn't be blocked for other users.
EXPECT_FALSE(form_fetcher_->IsMovingBlocked(kAnotherUser,
blocked_form.username_value));
// PSL match entries should be ignored when computing the moving blacklist
// entries.
EXPECT_FALSE(form_fetcher_->IsMovingBlocked(kUser, psl_form.username_value));
}
} // 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