Commit 259f710b authored by edchin's avatar edchin Committed by Commit Bot

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

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: I442c85fd17528bc05be8505f0e3a482823daeae0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2538196Reviewed-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@{#827806}
parent ea154b63
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include <memory> #include <memory>
#include <utility> #include <utility>
#include "base/feature_list.h"
#include "base/metrics/field_trial.h" #include "base/metrics/field_trial.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/strings/strcat.h" #include "base/strings/strcat.h"
...@@ -63,9 +64,7 @@ using autofill::USERNAME; ...@@ -63,9 +64,7 @@ using autofill::USERNAME;
using base::NumberToString; using base::NumberToString;
using BlacklistedStatus = using BlacklistedStatus =
password_manager::OriginCredentialStore::BlacklistedStatus; password_manager::OriginCredentialStore::BlacklistedStatus;
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
using password_manager::metrics_util::GaiaPasswordHashChange; using password_manager::metrics_util::GaiaPasswordHashChange;
#endif // PASSWORD_REUSE_DETECTION_ENABLED
namespace password_manager { namespace password_manager {
...@@ -974,7 +973,10 @@ void PasswordManager::OnLoginSuccessful() { ...@@ -974,7 +973,10 @@ void PasswordManager::OnLoginSuccessful() {
void PasswordManager::MaybeSavePasswordHash( void PasswordManager::MaybeSavePasswordHash(
PasswordFormManager* submitted_manager) { PasswordFormManager* submitted_manager) {
#if defined(PASSWORD_REUSE_DETECTION_ENABLED) if (!base::FeatureList::IsEnabled(features::kPasswordReuseDetectionEnabled)) {
return;
}
const PasswordForm* submitted_form = submitted_manager->GetSubmittedForm(); const PasswordForm* submitted_form = submitted_manager->GetSubmittedForm();
// When |username_value| is empty, it's not clear whether the submitted // When |username_value| is empty, it's not clear whether the submitted
// credentials are really Gaia or enterprise credentials. Don't save // credentials are really Gaia or enterprise credentials. Don't save
...@@ -1033,7 +1035,6 @@ void PasswordManager::MaybeSavePasswordHash( ...@@ -1033,7 +1035,6 @@ void PasswordManager::MaybeSavePasswordHash(
? GaiaPasswordHashChange::NOT_SYNC_PASSWORD_CHANGE ? GaiaPasswordHashChange::NOT_SYNC_PASSWORD_CHANGE
: GaiaPasswordHashChange::SAVED_IN_CONTENT_AREA); : GaiaPasswordHashChange::SAVED_IN_CONTENT_AREA);
store->SaveGaiaPasswordHash(username, password, is_sync_account_email, event); store->SaveGaiaPasswordHash(username, password, is_sync_account_email, event);
#endif
} }
void PasswordManager::ProcessAutofillPredictions( void PasswordManager::ProcessAutofillPredictions(
......
...@@ -340,11 +340,14 @@ class PasswordManagerTest : public testing::TestWithParam<bool> { ...@@ -340,11 +340,14 @@ class PasswordManagerTest : public testing::TestWithParam<bool> {
PasswordManagerTest() : task_runner_(new TestMockTimeTaskRunner) { PasswordManagerTest() : task_runner_(new TestMockTimeTaskRunner) {
bool enable_passwords_account_storage = GetParam(); bool enable_passwords_account_storage = GetParam();
if (enable_passwords_account_storage) { if (enable_passwords_account_storage) {
feature_list_.InitAndEnableFeature( feature_list_.InitWithFeatures(
features::kEnablePasswordsAccountStorage); /*enable_features=*/{features::kPasswordReuseDetectionEnabled,
features::kEnablePasswordsAccountStorage},
/*disable_features=*/{});
} else { } else {
feature_list_.InitAndDisableFeature( feature_list_.InitWithFeatures(
features::kEnablePasswordsAccountStorage); /*enable_features=*/{features::kPasswordReuseDetectionEnabled},
/*disable_features=*/{features::kEnablePasswordsAccountStorage});
} }
} }
~PasswordManagerTest() override = default; ~PasswordManagerTest() override = default;
...@@ -1277,17 +1280,17 @@ TEST_P(PasswordManagerTest, SyncCredentialsNotSaved) { ...@@ -1277,17 +1280,17 @@ TEST_P(PasswordManagerTest, SyncCredentialsNotSaved) {
// User should not be prompted and password should not be saved. // User should not be prompted and password should not be saved.
EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0); EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr(_)).Times(0);
EXPECT_CALL(*store_, AddLogin(_)).Times(0); EXPECT_CALL(*store_, AddLogin(_)).Times(0);
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
ON_CALL(*client_.GetStoreResultFilter(), ShouldSaveGaiaPasswordHash(_)) ON_CALL(*client_.GetStoreResultFilter(), ShouldSaveGaiaPasswordHash(_))
.WillByDefault(Return(true)); .WillByDefault(Return(true));
ON_CALL(*client_.GetStoreResultFilter(), IsSyncAccountEmail(_)) ON_CALL(*client_.GetStoreResultFilter(), IsSyncAccountEmail(_))
.WillByDefault(Return(true)); .WillByDefault(Return(true));
EXPECT_CALL( EXPECT_CALL(*store_,
*store_, SaveGaiaPasswordHash(
SaveGaiaPasswordHash( "googleuser", form_data.fields[1].value,
"googleuser", form_data.fields[1].value, /*is_primary_account=*/true, /*is_primary_account=*/true,
metrics_util::GaiaPasswordHashChange::SAVED_IN_CONTENT_AREA)); metrics_util::GaiaPasswordHashChange::SAVED_IN_CONTENT_AREA));
#endif
EXPECT_CALL(client_, IsSavingAndFillingEnabled(form_data.url)) EXPECT_CALL(client_, IsSavingAndFillingEnabled(form_data.url))
.WillRepeatedly(Return(true)); .WillRepeatedly(Return(true));
...@@ -1299,7 +1302,6 @@ TEST_P(PasswordManagerTest, SyncCredentialsNotSaved) { ...@@ -1299,7 +1302,6 @@ TEST_P(PasswordManagerTest, SyncCredentialsNotSaved) {
manager()->OnPasswordFormsRendered(&driver_, observed, true); manager()->OnPasswordFormsRendered(&driver_, observed, true);
} }
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
TEST_P(PasswordManagerTest, HashSavedOnGaiaFormWithSkipSavePassword) { TEST_P(PasswordManagerTest, HashSavedOnGaiaFormWithSkipSavePassword) {
for (bool did_stop_loading : {false, true}) { for (bool did_stop_loading : {false, true}) {
SCOPED_TRACE(testing::Message("did_stop_loading = ") << did_stop_loading); SCOPED_TRACE(testing::Message("did_stop_loading = ") << did_stop_loading);
...@@ -1373,7 +1375,6 @@ TEST_P(PasswordManagerTest, ...@@ -1373,7 +1375,6 @@ TEST_P(PasswordManagerTest,
OnPasswordFormSubmitted(form_data); OnPasswordFormSubmitted(form_data);
manager()->DidNavigateMainFrame(false); manager()->DidNavigateMainFrame(false);
} }
#endif
// On a successful login with an updated password, // On a successful login with an updated password,
// CredentialsFilter::ReportFormLoginSuccess and CredentialsFilter::ShouldSave // CredentialsFilter::ReportFormLoginSuccess and CredentialsFilter::ShouldSave
...@@ -1436,7 +1437,6 @@ TEST_P(PasswordManagerTest, SyncCredentialsNotDroppedIfUpToDate) { ...@@ -1436,7 +1437,6 @@ TEST_P(PasswordManagerTest, SyncCredentialsNotDroppedIfUpToDate) {
EXPECT_CALL(client_, IsSavingAndFillingEnabled(form.url)) EXPECT_CALL(client_, IsSavingAndFillingEnabled(form.url))
.WillRepeatedly(Return(true)); .WillRepeatedly(Return(true));
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
ON_CALL(*client_.GetStoreResultFilter(), ShouldSaveGaiaPasswordHash(_)) ON_CALL(*client_.GetStoreResultFilter(), ShouldSaveGaiaPasswordHash(_))
.WillByDefault(Return(true)); .WillByDefault(Return(true));
ON_CALL(*client_.GetStoreResultFilter(), IsSyncAccountEmail(_)) ON_CALL(*client_.GetStoreResultFilter(), IsSyncAccountEmail(_))
...@@ -1446,7 +1446,7 @@ TEST_P(PasswordManagerTest, SyncCredentialsNotDroppedIfUpToDate) { ...@@ -1446,7 +1446,7 @@ TEST_P(PasswordManagerTest, SyncCredentialsNotDroppedIfUpToDate) {
SaveGaiaPasswordHash( SaveGaiaPasswordHash(
"googleuser", form.password_value, /*is_primary_account=*/true, "googleuser", form.password_value, /*is_primary_account=*/true,
metrics_util::GaiaPasswordHashChange::SAVED_IN_CONTENT_AREA)); metrics_util::GaiaPasswordHashChange::SAVED_IN_CONTENT_AREA));
#endif
manager()->OnPasswordFormSubmitted(&driver_, form.form_data); manager()->OnPasswordFormSubmitted(&driver_, form.form_data);
// Chrome should not remove the sync credential, because it was successfully // Chrome should not remove the sync credential, because it was successfully
...@@ -2313,7 +2313,6 @@ TEST_P(PasswordManagerTest, ClearedFieldsSuccessCriteria) { ...@@ -2313,7 +2313,6 @@ TEST_P(PasswordManagerTest, ClearedFieldsSuccessCriteria) {
manager()->OnPasswordFormsRendered(&driver_, observed, true); manager()->OnPasswordFormsRendered(&driver_, observed, true);
} }
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
// Check that no sync password hash is saved when no username is available, // Check that no sync password hash is saved when no username is available,
// because we it's not clear whether the submitted credentials are sync // because we it's not clear whether the submitted credentials are sync
// credentials. // credentials.
...@@ -2366,7 +2365,6 @@ TEST_P(PasswordManagerTest, NotSavingSyncPasswordHash_NotSyncCredentials) { ...@@ -2366,7 +2365,6 @@ TEST_P(PasswordManagerTest, NotSavingSyncPasswordHash_NotSyncCredentials) {
observed.clear(); observed.clear();
manager()->OnPasswordFormsRendered(&driver_, observed, true); manager()->OnPasswordFormsRendered(&driver_, observed, true);
} }
#endif
TEST_P(PasswordManagerTest, ManualFallbackForSaving) { TEST_P(PasswordManagerTest, ManualFallbackForSaving) {
ukm::TestAutoSetUkmRecorder test_ukm_recorder; ukm::TestAutoSetUkmRecorder test_ukm_recorder;
...@@ -2509,7 +2507,6 @@ TEST_P(PasswordManagerTest, SaveSyncPasswordHashOnChangePasswordPage) { ...@@ -2509,7 +2507,6 @@ TEST_P(PasswordManagerTest, SaveSyncPasswordHashOnChangePasswordPage) {
EXPECT_CALL(client_, IsSavingAndFillingEnabled(form_data.url)) EXPECT_CALL(client_, IsSavingAndFillingEnabled(form_data.url))
.WillRepeatedly(Return(true)); .WillRepeatedly(Return(true));
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
ON_CALL(*client_.GetStoreResultFilter(), ShouldSaveGaiaPasswordHash(_)) ON_CALL(*client_.GetStoreResultFilter(), ShouldSaveGaiaPasswordHash(_))
.WillByDefault(Return(true)); .WillByDefault(Return(true));
ON_CALL(*client_.GetStoreResultFilter(), IsSyncAccountEmail(_)) ON_CALL(*client_.GetStoreResultFilter(), IsSyncAccountEmail(_))
...@@ -2517,9 +2514,10 @@ TEST_P(PasswordManagerTest, SaveSyncPasswordHashOnChangePasswordPage) { ...@@ -2517,9 +2514,10 @@ TEST_P(PasswordManagerTest, SaveSyncPasswordHashOnChangePasswordPage) {
EXPECT_CALL( EXPECT_CALL(
*store_, *store_,
SaveGaiaPasswordHash( SaveGaiaPasswordHash(
"googleuser", form_data.fields[1].value, /*is_primary_account=*/true, "googleuser", form_data.fields[1].value,
/*is_primary_account=*/true,
metrics_util::GaiaPasswordHashChange::CHANGED_IN_CONTENT_AREA)); metrics_util::GaiaPasswordHashChange::CHANGED_IN_CONTENT_AREA));
#endif
client_.FilterAllResultsForSaving(); client_.FilterAllResultsForSaving();
OnPasswordFormSubmitted(form_data); OnPasswordFormSubmitted(form_data);
...@@ -2528,7 +2526,6 @@ TEST_P(PasswordManagerTest, SaveSyncPasswordHashOnChangePasswordPage) { ...@@ -2528,7 +2526,6 @@ TEST_P(PasswordManagerTest, SaveSyncPasswordHashOnChangePasswordPage) {
manager()->OnPasswordFormsRendered(&driver_, observed, true); manager()->OnPasswordFormsRendered(&driver_, observed, true);
} }
#if defined(PASSWORD_REUSE_DETECTION_ENABLED)
// Non-Sync Gaia password hash should be saved upon submission of Gaia login // Non-Sync Gaia password hash should be saved upon submission of Gaia login
// page. // page.
TEST_P(PasswordManagerTest, SaveOtherGaiaPasswordHash) { TEST_P(PasswordManagerTest, SaveOtherGaiaPasswordHash) {
...@@ -2618,7 +2615,6 @@ TEST_P(PasswordManagerTest, SaveEnterprisePasswordHash) { ...@@ -2618,7 +2615,6 @@ TEST_P(PasswordManagerTest, SaveEnterprisePasswordHash) {
observed.clear(); observed.clear();
manager()->OnPasswordFormsRendered(&driver_, observed, true); manager()->OnPasswordFormsRendered(&driver_, observed, true);
} }
#endif
// If there are no forms to parse, certificate errors should not be reported. // If there are no forms to parse, certificate errors should not be reported.
TEST_P(PasswordManagerTest, CertErrorReported_NoForms) { TEST_P(PasswordManagerTest, CertErrorReported_NoForms) {
......
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