Commit f609fffc authored by edchin's avatar edchin Committed by Commit Bot

[ios][PhishGuard] Compile password reuse detection code in password_store

Design doc: go/bling-phishguard

This CL continues work to compile code in iOS that was previously
disabled behind PASSWORD_REUSE_DETECTION_ENABLED.

Runtime changes are put behind a feature flag.

Bug: 1147962
Change-Id: I4cd821e3bf80dbd6a68d393dfb16510d11e78940
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537916Reviewed-by: default avatarAli Juma <ajuma@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827802}
parent 6692b0d8
...@@ -12,12 +12,14 @@ ...@@ -12,12 +12,14 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/callback_helpers.h" #include "base/callback_helpers.h"
#include "base/debug/dump_without_crashing.h" #include "base/debug/dump_without_crashing.h"
#include "base/feature_list.h"
#include "base/location.h" #include "base/location.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/string16.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/task/thread_pool.h" #include "base/task/thread_pool.h"
#include "base/task_runner_util.h" #include "base/task_runner_util.h"
...@@ -34,20 +36,24 @@ ...@@ -34,20 +36,24 @@
#include "components/password_manager/core/browser/password_manager_metrics_util.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/password_manager/core/browser/password_manager_util.h" #include "components/password_manager/core/browser/password_manager_util.h"
#include "components/password_manager/core/browser/password_store_consumer.h" #include "components/password_manager/core/browser/password_store_consumer.h"
#include "components/password_manager/core/browser/password_store_signin_notifier.h"
#include "components/password_manager/core/browser/statistics_table.h" #include "components/password_manager/core/browser/statistics_table.h"
#include "components/password_manager/core/browser/sync/password_sync_bridge.h" #include "components/password_manager/core/browser/sync/password_sync_bridge.h"
#include "components/password_manager/core/common/password_manager_features.h"
#include "components/password_manager/core/common/password_manager_pref_names.h" #include "components/password_manager/core/common/password_manager_pref_names.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
#include "components/sync/model_impl/client_tag_based_model_type_processor.h" #include "components/sync/model_impl/client_tag_based_model_type_processor.h"
#include "components/sync/model_impl/proxy_model_type_controller_delegate.h" #include "components/sync/model_impl/proxy_model_type_controller_delegate.h"
#include "base/strings/string16.h"
#include "components/password_manager/core/browser/password_store_signin_notifier.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
namespace password_manager { namespace password_manager {
namespace { namespace {
bool IsPasswordReuseDetectionEnabled() {
return base::FeatureList::IsEnabled(features::kPasswordReuseDetectionEnabled);
}
// Utility function to simplify removing logins prior a given |cutoff| data. // Utility function to simplify removing logins prior a given |cutoff| data.
// Runs |callback| with the result. // Runs |callback| with the result.
// //
...@@ -156,9 +162,10 @@ bool PasswordStore::Init(PrefService* prefs, ...@@ -156,9 +162,10 @@ bool PasswordStore::Init(PrefService* prefs,
DCHECK(background_task_runner_); DCHECK(background_task_runner_);
sync_enabled_or_disabled_cb_ = std::move(sync_enabled_or_disabled_cb); sync_enabled_or_disabled_cb_ = std::move(sync_enabled_or_disabled_cb);
prefs_ = prefs; prefs_ = prefs;
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
hash_password_manager_.set_prefs(prefs); if (IsPasswordReuseDetectionEnabled()) {
#endif hash_password_manager_.set_prefs(prefs);
}
if (background_task_runner_) { if (background_task_runner_) {
TRACE_EVENT_NESTABLE_ASYNC_BEGIN0( TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(
"passwords", "PasswordStore::InitOnBackgroundSequence", this); "passwords", "PasswordStore::InitOnBackgroundSequence", this);
...@@ -339,8 +346,7 @@ void PasswordStore::ReportMetrics(const std::string& sync_username, ...@@ -339,8 +346,7 @@ void PasswordStore::ReportMetrics(const std::string& sync_username,
base::TimeDelta::FromSeconds(30)); base::TimeDelta::FromSeconds(30));
} }
#if defined(PASSWORD_REUSE_DETECTION_ENABLED) if (IsPasswordReuseDetectionEnabled() && !sync_username.empty()) {
if (!sync_username.empty()) {
auto hash_password_state = auto hash_password_state =
hash_password_manager_.HasPasswordHash(sync_username, hash_password_manager_.HasPasswordHash(sync_username,
/*is_gaia_password=*/true) /*is_gaia_password=*/true)
...@@ -349,7 +355,6 @@ void PasswordStore::ReportMetrics(const std::string& sync_username, ...@@ -349,7 +355,6 @@ void PasswordStore::ReportMetrics(const std::string& sync_username,
metrics_util::LogIsSyncPasswordHashSaved(hash_password_state, metrics_util::LogIsSyncPasswordHashSaved(hash_password_state,
is_under_advanced_protection); is_under_advanced_protection);
} }
#endif
} }
void PasswordStore::AddSiteStats(const InteractionsStats& stats) { void PasswordStore::AddSiteStats(const InteractionsStats& stats) {
...@@ -515,10 +520,8 @@ void PasswordStore::ShutdownOnUIThread() { ...@@ -515,10 +520,8 @@ void PasswordStore::ShutdownOnUIThread() {
// The AffiliationService must be destroyed from the main sequence. // The AffiliationService must be destroyed from the main sequence.
affiliated_match_helper_.reset(); affiliated_match_helper_.reset();
shutdown_called_ = true; shutdown_called_ = true;
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
if (notifier_) if (notifier_)
notifier_->UnsubscribeFromSigninEvents(); notifier_->UnsubscribeFromSigninEvents();
#endif
} }
std::unique_ptr<syncer::ProxyModelTypeControllerDelegate> std::unique_ptr<syncer::ProxyModelTypeControllerDelegate>
...@@ -689,15 +692,15 @@ bool PasswordStore::InitOnBackgroundSequence() { ...@@ -689,15 +692,15 @@ bool PasswordStore::InitOnBackgroundSequence() {
syncer::PASSWORDS, base::DoNothing()), syncer::PASSWORDS, base::DoNothing()),
/*password_store_sync=*/this, sync_enabled_or_disabled_cb_)); /*password_store_sync=*/this, sync_enabled_or_disabled_cb_));
#if defined(PASSWORD_REUSE_DETECTION_ENABLED) if (IsPasswordReuseDetectionEnabled()) {
reuse_detector_ = new PasswordReuseDetector; reuse_detector_ = new PasswordReuseDetector;
base::SequencedTaskRunnerHandle::Get()->PostTask( base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce(&PasswordStoreConsumer::OnGetPasswordStoreResultsFrom, base::BindOnce(&PasswordStoreConsumer::OnGetPasswordStoreResultsFrom,
reuse_detector_->GetWeakPtr(), base::RetainedRef(this), reuse_detector_->GetWeakPtr(), base::RetainedRef(this),
GetAutofillableLoginsImpl())); GetAutofillableLoginsImpl()));
#endif }
return true; return true;
} }
...@@ -744,10 +747,9 @@ void PasswordStore::NotifyLoginsChanged( ...@@ -744,10 +747,9 @@ void PasswordStore::NotifyLoginsChanged(
if (sync_bridge_) if (sync_bridge_)
sync_bridge_->ActOnPasswordStoreChanges(changes); sync_bridge_->ActOnPasswordStoreChanges(changes);
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
if (reuse_detector_) if (reuse_detector_)
reuse_detector_->OnLoginsChanged(changes); reuse_detector_->OnLoginsChanged(changes);
#endif
ProcessLoginsChanged( ProcessLoginsChanged(
changes, changes,
base::BindRepeating( base::BindRepeating(
...@@ -1373,10 +1375,8 @@ void PasswordStore::DestroyOnBackgroundSequence() { ...@@ -1373,10 +1375,8 @@ void PasswordStore::DestroyOnBackgroundSequence() {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence()); DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
sync_bridge_.reset(); sync_bridge_.reset();
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
delete reuse_detector_; delete reuse_detector_;
reuse_detector_ = nullptr; reuse_detector_ = nullptr;
#endif
} }
} // namespace password_manager } // namespace password_manager
...@@ -137,6 +137,9 @@ class PasswordStoreTest : public testing::Test { ...@@ -137,6 +137,9 @@ class PasswordStoreTest : public testing::Test {
// Mock OSCrypt. There is a call to OSCrypt on initializling // Mock OSCrypt. There is a call to OSCrypt on initializling
// PasswordReuseDetector, so it should be mocked. // PasswordReuseDetector, so it should be mocked.
OSCryptMocker::SetUp(); OSCryptMocker::SetUp();
feature_list_.InitAndEnableFeature(
features::kPasswordReuseDetectionEnabled);
} }
void TearDown() override { OSCryptMocker::TearDown(); } void TearDown() override { OSCryptMocker::TearDown(); }
...@@ -156,6 +159,7 @@ class PasswordStoreTest : public testing::Test { ...@@ -156,6 +159,7 @@ class PasswordStoreTest : public testing::Test {
base::ScopedTempDir temp_dir_; base::ScopedTempDir temp_dir_;
base::test::TaskEnvironment task_environment_{ base::test::TaskEnvironment task_environment_{
base::test::TaskEnvironment::MainThreadType::UI}; base::test::TaskEnvironment::MainThreadType::UI};
base::test::ScopedFeatureList feature_list_;
DISALLOW_COPY_AND_ASSIGN(PasswordStoreTest); DISALLOW_COPY_AND_ASSIGN(PasswordStoreTest);
}; };
...@@ -1179,7 +1183,6 @@ TEST_F(PasswordStoreTest, Unblacklisting) { ...@@ -1179,7 +1183,6 @@ TEST_F(PasswordStoreTest, Unblacklisting) {
store->ShutdownOnUIThread(); store->ShutdownOnUIThread();
} }
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
TEST_F(PasswordStoreTest, CheckPasswordReuse) { TEST_F(PasswordStoreTest, CheckPasswordReuse) {
static constexpr PasswordFormData kTestCredentials[] = { static constexpr PasswordFormData kTestCredentials[] = {
{PasswordForm::Scheme::kHtml, "https://www.google.com", {PasswordForm::Scheme::kHtml, "https://www.google.com",
...@@ -1460,7 +1463,6 @@ TEST_F(PasswordStoreTest, ReportMetricsForNonSyncPassword) { ...@@ -1460,7 +1463,6 @@ TEST_F(PasswordStoreTest, ReportMetricsForNonSyncPassword) {
GaiaPasswordHashChange::NOT_SYNC_PASSWORD_CHANGE, 1); GaiaPasswordHashChange::NOT_SYNC_PASSWORD_CHANGE, 1);
store->ShutdownOnUIThread(); store->ShutdownOnUIThread();
} }
#endif
TEST_F(PasswordStoreTest, GetAllCompromisedCredentials) { TEST_F(PasswordStoreTest, GetAllCompromisedCredentials) {
base::test::ScopedFeatureList feature_list; base::test::ScopedFeatureList feature_list;
......
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