Skip to content

Commit

Permalink
Add metrics for non sync users; show non sync warning dialog.
Browse files Browse the repository at this point in the history
This is a follow up CL after cr/1673863 and cr/1719975.
The CL is large because adding functionality for non sync users
require retrieving and passing around the username. Previously,
the primary account is easily accessed. Now, to get the non syncing
accounts, all the signed in accounts needs to be traversed to figure
out which account password was reused.

This CL includes several functionality:
-Recording metrics for non-sync users
-Applies the same functionality as the primary account to the
content area accounts
-Add two malicious states and two safe browsing states that replaces
the original SIGN_IN_PASSWORD_REUSE state
-Refactored most methods to take in ReusedPasswordAccountType instead
of PasswordType and username.
-Changed kSafeBrowsingUnhandledSyncPasswordReuses to
kSafeBrowsingUnhandledGaiaPasswordReuses
-Saves a username variable inside the ChromePasswordProtectionService
in order to retrieve the content area account info to see if the
account is GMAIL, GSUITE, syncing, etc.

Bug: 914410
Change-Id: I86d807c6fb25c5bb6e9075411be44bf42451a859
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1693188
Commit-Queue: Bettina Dea <bdea@chromium.org>
Reviewed-by: Owen Min <zmin@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686232}
  • Loading branch information
Bettina authored and Commit Bot committed Aug 12, 2019
1 parent 6970d0e commit 109c27a
Show file tree
Hide file tree
Showing 40 changed files with 1,415 additions and 836 deletions.
3 changes: 2 additions & 1 deletion chrome/browser/interstitials/enterprise_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ std::string GetThreatTypeStringForInterstitial(
case safe_browsing::SB_THREAT_TYPE_CSD_WHITELIST:
case safe_browsing::
DEPRECATED_SB_THREAT_TYPE_URL_PASSWORD_PROTECTION_PHISHING:
case safe_browsing::SB_THREAT_TYPE_SIGN_IN_PASSWORD_REUSE:
case safe_browsing::SB_THREAT_TYPE_SIGNED_IN_SYNC_PASSWORD_REUSE:
case safe_browsing::SB_THREAT_TYPE_SIGNED_IN_NON_SYNC_PASSWORD_REUSE:
case safe_browsing::SB_THREAT_TYPE_AD_SAMPLE:
case safe_browsing::SB_THREAT_TYPE_BLOCKED_AD_POPUP:
case safe_browsing::SB_THREAT_TYPE_BLOCKED_AD_REDIRECT:
Expand Down
38 changes: 19 additions & 19 deletions chrome/browser/policy/policy_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@

using content::BrowserThread;
using net::URLRequestMockHTTPJob;
using safe_browsing::ReusedPasswordAccountType;
using testing::_;
using testing::Mock;
using testing::Return;
Expand Down Expand Up @@ -754,10 +755,6 @@ class MockPasswordProtectionService
: safe_browsing::ChromePasswordProtectionService(sb_service, profile) {}
~MockPasswordProtectionService() override {}

MOCK_CONST_METHOD0(GetSyncAccountType,
safe_browsing::LoginReputationClientRequest::
PasswordReuseEvent::SyncAccountType());

MOCK_CONST_METHOD0(IsPrimaryAccountGmail, bool());

AccountInfo GetAccountInfo() const override {
Expand Down Expand Up @@ -5023,14 +5020,12 @@ IN_PROC_BROWSER_TEST_F(PolicyTest,

// If user is not signed-in, |GetPasswordProtectionWarningTriggerPref(...)|
// should return |PHISHING_REUSE| unless specified by policy.
EXPECT_CALL(mock_service, GetSyncAccountType())
.WillRepeatedly(Return(safe_browsing::LoginReputationClientRequest::
PasswordReuseEvent::NOT_SIGNED_IN));
const PrefService* const prefs = browser()->profile()->GetPrefs();
EXPECT_FALSE(prefs->FindPreference(prefs::kPasswordProtectionWarningTrigger)
->IsManaged());
EXPECT_EQ(safe_browsing::PHISHING_REUSE,
mock_service.GetPasswordProtectionWarningTriggerPref());
mock_service.GetPasswordProtectionWarningTriggerPref(
ReusedPasswordAccountType()));
// Sets the enterprise policy to 1 (a.k.a PASSWORD_REUSE).
PolicyMap policies;
policies.Set(key::kPasswordProtectionWarningTrigger, POLICY_LEVEL_MANDATORY,
Expand All @@ -5040,14 +5035,16 @@ IN_PROC_BROWSER_TEST_F(PolicyTest,
EXPECT_TRUE(prefs->FindPreference(prefs::kPasswordProtectionWarningTrigger)
->IsManaged());
EXPECT_EQ(safe_browsing::PASSWORD_REUSE,
mock_service.GetPasswordProtectionWarningTriggerPref());
mock_service.GetPasswordProtectionWarningTriggerPref(
ReusedPasswordAccountType()));
// Sets the enterprise policy to 2 (a.k.a PHISHING_REUSE).
policies.Set(key::kPasswordProtectionWarningTrigger, POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD,
std::make_unique<base::Value>(2), nullptr);
UpdateProviderPolicy(policies);
EXPECT_EQ(safe_browsing::PHISHING_REUSE,
mock_service.GetPasswordProtectionWarningTriggerPref());
mock_service.GetPasswordProtectionWarningTriggerPref(
ReusedPasswordAccountType()));
}

// Test that when password protection warning trigger is set for Gmail users,
Expand All @@ -5064,8 +5061,11 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, PasswordProtectionWarningTriggerGmail) {
const PrefService* const prefs = browser()->profile()->GetPrefs();
EXPECT_FALSE(prefs->FindPreference(prefs::kPasswordProtectionWarningTrigger)
->IsManaged());
ReusedPasswordAccountType account_type;
account_type.set_account_type(ReusedPasswordAccountType::GMAIL);
account_type.set_is_account_syncing(true);
EXPECT_EQ(safe_browsing::PHISHING_REUSE,
mock_service.GetPasswordProtectionWarningTriggerPref());
mock_service.GetPasswordProtectionWarningTriggerPref(account_type));
// Sets the enterprise policy to 1 (a.k.a PASSWORD_REUSE). Gmail accounts
// should always return PHISHING_REUSE regardless of what the policy is set
// to.
Expand All @@ -5077,24 +5077,21 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, PasswordProtectionWarningTriggerGmail) {
EXPECT_TRUE(prefs->FindPreference(prefs::kPasswordProtectionWarningTrigger)
->IsManaged());
EXPECT_EQ(safe_browsing::PHISHING_REUSE,
mock_service.GetPasswordProtectionWarningTriggerPref());
mock_service.GetPasswordProtectionWarningTriggerPref(account_type));
// Sets the enterprise policy to 2 (a.k.a PHISHING_REUSE).
policies.Set(key::kPasswordProtectionWarningTrigger, POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD,
std::make_unique<base::Value>(2), nullptr);
UpdateProviderPolicy(policies);
EXPECT_EQ(safe_browsing::PHISHING_REUSE,
mock_service.GetPasswordProtectionWarningTriggerPref());
mock_service.GetPasswordProtectionWarningTriggerPref(account_type));
}

// Test that when password protection warning trigger is set for GSuite users,
// Chrome password protection service gets the correct value.
IN_PROC_BROWSER_TEST_F(PolicyTest, PasswordProtectionWarningTriggerGSuite) {
MockPasswordProtectionService mock_service(
g_browser_process->safe_browsing_service(), browser()->profile());
EXPECT_CALL(mock_service, GetSyncAccountType())
.WillRepeatedly(Return(safe_browsing::LoginReputationClientRequest::
PasswordReuseEvent::GSUITE));
const PrefService* const prefs = browser()->profile()->GetPrefs();
PolicyMap policies;

Expand All @@ -5103,7 +5100,8 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, PasswordProtectionWarningTriggerGSuite) {
EXPECT_FALSE(prefs->FindPreference(prefs::kPasswordProtectionWarningTrigger)
->IsManaged());
EXPECT_EQ(safe_browsing::PHISHING_REUSE,
mock_service.GetPasswordProtectionWarningTriggerPref());
mock_service.GetPasswordProtectionWarningTriggerPref(
ReusedPasswordAccountType()));
// Sets the enterprise policy to 1 (a.k.a PASSWORD_REUSE).
policies.Set(key::kPasswordProtectionWarningTrigger, POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD,
Expand All @@ -5112,14 +5110,16 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, PasswordProtectionWarningTriggerGSuite) {
EXPECT_TRUE(prefs->FindPreference(prefs::kPasswordProtectionWarningTrigger)
->IsManaged());
EXPECT_EQ(safe_browsing::PASSWORD_REUSE,
mock_service.GetPasswordProtectionWarningTriggerPref());
mock_service.GetPasswordProtectionWarningTriggerPref(
ReusedPasswordAccountType()));
// Sets the enterprise policy to 2 (a.k.a PHISHING_REUSE).
policies.Set(key::kPasswordProtectionWarningTrigger, POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD,
std::make_unique<base::Value>(2), nullptr);
UpdateProviderPolicy(policies);
EXPECT_EQ(safe_browsing::PHISHING_REUSE,
mock_service.GetPasswordProtectionWarningTriggerPref());
mock_service.GetPasswordProtectionWarningTriggerPref(
ReusedPasswordAccountType()));
}

// Test that when safe browsing whitelist domains are set by policy, safe
Expand Down
Loading

0 comments on commit 109c27a

Please sign in to comment.