Commit df863e5c authored by Bettina's avatar Bettina Committed by Commit Bot

Apply SBER checks to Enhanced Protection for Password Protection

As Enhanced Protection is a superset of SBER,
everywhere that checks for SBER should also
have a check for enhanced protection.

Bug: 1054254
Change-Id: Ie8a89d450d0a6b7fd823fb4dc802ffe8bb1a27dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2065858
Commit-Queue: Bettina Dea <bdea@chromium.org>
Reviewed-by: default avatarDaniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743382}
parent 3782065d
......@@ -1393,6 +1393,10 @@ bool ChromePasswordProtectionService::IsExtendedReporting() {
return IsExtendedReportingEnabled(*GetPrefs());
}
bool ChromePasswordProtectionService::IsEnhancedProtection() {
return IsEnhancedProtectionEnabled(*GetPrefs());
}
bool ChromePasswordProtectionService::IsIncognito() {
return profile_->IsOffTheRecord();
}
......@@ -1406,10 +1410,12 @@ bool ChromePasswordProtectionService::IsPingingEnabled(
return false;
}
bool extended_reporting_enabled = IsExtendedReporting();
bool enhanced_protection_enabled = IsEnhancedProtection();
if (trigger_type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT) {
if (password_type.account_type() ==
ReusedPasswordAccountType::SAVED_PASSWORD) {
bool enabled = extended_reporting_enabled ||
enhanced_protection_enabled ||
base::FeatureList::IsEnabled(
safe_browsing::kPasswordProtectionForSavedPasswords);
if (!enabled)
......@@ -1427,13 +1433,13 @@ bool ChromePasswordProtectionService::IsPingingEnabled(
// If the account type is UNKNOWN (i.e. AccountInfo fields could not be
// retrieved from server), pings should be gated by SBER.
if (password_type.account_type() == ReusedPasswordAccountType::UNKNOWN) {
return extended_reporting_enabled;
return extended_reporting_enabled || enhanced_protection_enabled;
}
// Only saved password reuse warnings are shown on Android, so other types of
// password reuse events should be gated by extended reporting.
#if defined(OS_ANDROID)
return extended_reporting_enabled;
return extended_reporting_enabled || enhanced_protection_enabled;
#else
return true;
#endif
......@@ -1445,7 +1451,7 @@ bool ChromePasswordProtectionService::IsPingingEnabled(
*reason = RequestOutcome::DISABLED_DUE_TO_INCOGNITO;
return false;
}
if (!extended_reporting_enabled) {
if (!extended_reporting_enabled && !enhanced_protection_enabled) {
*reason = RequestOutcome::DISABLED_DUE_TO_USER_POPULATION;
return false;
}
......@@ -1679,7 +1685,7 @@ void ChromePasswordProtectionService::SanitizeReferrerChain(
bool ChromePasswordProtectionService::CanSendSamplePing() {
// Send a sample ping only 1% of the time.
return IsExtendedReporting() && !IsIncognito() &&
return (IsExtendedReporting() || IsEnhancedProtection()) && !IsIncognito() &&
(bypass_probability_for_tests_ ||
base::RandDouble() <= kProbabilityForSendingReportsFromSafeURLs);
}
......
......@@ -289,6 +289,8 @@ class ChromePasswordProtectionService : public PasswordProtectionService {
bool IsExtendedReporting() override;
bool IsEnhancedProtection() override;
bool IsIncognito() override;
// Checks if pinging should be enabled based on the |trigger_type|,
......
......@@ -155,8 +155,10 @@ class MockChromePasswordProtectionService
is_extended_reporting_(false),
is_syncing_(false),
is_no_hosted_domain_found_(false),
is_account_signed_in_(false) {}
is_account_signed_in_(false),
is_enhanced_protection_(false) {}
bool IsExtendedReporting() override { return is_extended_reporting_; }
bool IsEnhancedProtection() override { return is_enhanced_protection_; }
bool IsIncognito() override { return is_incognito_; }
bool IsPrimaryAccountSyncing() const override { return is_syncing_; }
bool IsPrimaryAccountSignedIn() const override {
......@@ -184,6 +186,9 @@ class MockChromePasswordProtectionService
void SetIsAccountSignedIn(bool is_account_signed_in) {
is_account_signed_in_ = is_account_signed_in;
}
void SetIsEnhancedProtection(bool is_enhanced_protection) {
is_enhanced_protection_ = is_enhanced_protection;
}
void SetAccountInfo(const std::string& username) {
AccountInfo account_info;
account_info.account_id = CoreAccountId("account_id");
......@@ -203,6 +208,7 @@ class MockChromePasswordProtectionService
bool is_syncing_;
bool is_no_hosted_domain_found_;
bool is_account_signed_in_;
bool is_enhanced_protection_;
AccountInfo account_info_;
std::string mocked_sync_password_hash_;
};
......@@ -398,7 +404,8 @@ TEST_F(ChromePasswordProtectionServiceTest,
ReusedPasswordAccountType reused_password_type;
reused_password_type.set_account_type(ReusedPasswordAccountType::UNKNOWN);
// Password field on focus pinging is enabled on !incognito && SBER.
// Password field on focus pinging is enabled on !incognito && (SBER ||
// enhanced protection).
RequestOutcome reason;
service_->ConfigService(false /*incognito*/, false /*SBER*/);
EXPECT_FALSE(service_->IsPingingEnabled(
......@@ -406,6 +413,12 @@ TEST_F(ChromePasswordProtectionServiceTest,
&reason));
EXPECT_EQ(RequestOutcome::DISABLED_DUE_TO_USER_POPULATION, reason);
service_->SetIsEnhancedProtection(
/*is_enhanced_protection=*/true);
EXPECT_TRUE(service_->IsPingingEnabled(
LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE, reused_password_type,
&reason));
service_->ConfigService(false /*incognito*/, true /*SBER*/);
EXPECT_TRUE(service_->IsPingingEnabled(
LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE, reused_password_type,
......@@ -608,6 +621,17 @@ TEST_F(ChromePasswordProtectionServiceTest,
service_->PersistPhishedSavedPasswordCredential("username", domains);
}
TEST_F(ChromePasswordProtectionServiceTest,
VerifyPersistPhishedSavedPasswordCredentialForEnhancedProtection) {
service_->SetIsEnhancedProtection(
/*is_enhanced_protection=*/true);
std::vector<std::string> domains{"http://example.com",
"https://2.example.com"};
EXPECT_CALL(*password_store_, AddCompromisedCredentialsImpl(_)).Times(2);
service_->PersistPhishedSavedPasswordCredential("username", domains);
}
TEST_F(ChromePasswordProtectionServiceTest, VerifyCanSendSamplePing) {
// Experiment is on by default.
service_->ConfigService(/*is_incognito=*/false,
......@@ -615,19 +639,25 @@ TEST_F(ChromePasswordProtectionServiceTest, VerifyCanSendSamplePing) {
service_->set_bypass_probability_for_tests(true);
EXPECT_TRUE(service_->CanSendSamplePing());
// If not SBER, do not send sample ping.
service_->ConfigService(/*is_incognito=*/false,
/*is_extended_reporting=*/false);
EXPECT_FALSE(service_->CanSendSamplePing());
// If not SBER, do not send sample ping.
service_->ConfigService(/*is_incognito=*/false,
/*is_extended_reporting=*/false);
EXPECT_FALSE(service_->CanSendSamplePing());
// If incognito, do not send sample ping.
service_->ConfigService(/*is_incognito=*/true,
/*is_extended_reporting=*/true);
EXPECT_FALSE(service_->CanSendSamplePing());
// If incognito, do not send sample ping.
service_->ConfigService(/*is_incognito=*/true,
/*is_extended_reporting=*/true);
EXPECT_FALSE(service_->CanSendSamplePing());
service_->ConfigService(/*is_incognito=*/true,
/*is_extended_reporting=*/false);
EXPECT_FALSE(service_->CanSendSamplePing());
service_->ConfigService(/*is_incognito=*/true,
/*is_extended_reporting=*/false);
EXPECT_FALSE(service_->CanSendSamplePing());
service_->ConfigService(/*is_incognito=*/false,
/*is_extended_reporting=*/false);
service_->SetIsEnhancedProtection(
/*is_enhanced_protection=*/true);
EXPECT_TRUE(service_->CanSendSamplePing());
}
TEST_F(ChromePasswordProtectionServiceTest, VerifyGetOrganizationTypeGmail) {
......
......@@ -93,7 +93,8 @@ enum class RequestOutcome {
SERVICE_DESTROYED = 11,
// No request sent because pinging feature is disable.
DISABLED_DUE_TO_FEATURE_DISABLED = 12,
// No request sent because the user is not extended reporting user.
// No request sent because the user is not extended reporting or enhanced
// protection user.
DISABLED_DUE_TO_USER_POPULATION = 13,
// No request sent because the reputation of the URL is not computable.
URL_NOT_VALID_FOR_REPUTATION_COMPUTING = 14,
......
......@@ -41,6 +41,7 @@ class MockPasswordProtectionService : public PasswordProtectionService {
MOCK_METHOD0(CanSendSamplePing, bool());
MOCK_METHOD0(IsExtendedReporting, bool());
MOCK_METHOD0(IsEnhancedProtection, bool());
MOCK_METHOD0(IsIncognito, bool());
MOCK_METHOD0(IsHistorySyncEnabled, bool());
MOCK_METHOD0(IsUnderAdvancedProtection, bool());
......
......@@ -231,7 +231,8 @@ void PasswordProtectionRequest::FillRequestProto(bool is_sampled_ping) {
request_proto_->set_content_type(web_contents_->GetContentsMimeType());
#if BUILDFLAG(FULL_SAFE_BROWSING)
if (password_protection_service_->IsExtendedReporting() &&
if ((password_protection_service_->IsExtendedReporting() ||
password_protection_service_->IsEnhancedProtection()) &&
!password_protection_service_->IsIncognito()) {
gfx::Size content_area_size =
password_protection_service_->GetCurrentContentAreaSize();
......@@ -272,7 +273,8 @@ void PasswordProtectionRequest::FillRequestProto(bool is_sampled_ping) {
LogSyncAccountType(reuse_event->sync_account_type());
}
if ((password_protection_service_->IsExtendedReporting()) &&
if ((password_protection_service_->IsExtendedReporting() ||
password_protection_service_->IsEnhancedProtection()) &&
!password_protection_service_->IsIncognito()) {
for (const auto& domain : matching_domains_) {
reuse_event->add_domains_matching_password(domain);
......@@ -375,7 +377,8 @@ void PasswordProtectionRequest::MaybeCollectVisualFeatures() {
// Once the DOM features are collected, either collect visual features, or go
// straight to sending the ping.
if (trigger_type_ == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE &&
password_protection_service_->IsExtendedReporting() &&
(password_protection_service_->IsExtendedReporting() ||
password_protection_service_->IsEnhancedProtection()) &&
zoom::ZoomController::GetZoomLevelForWebContents(web_contents_) <=
kMaxZoomForVisualFeatures &&
request_proto_->content_area_width() >= kMinWidthForVisualFeatures &&
......
......@@ -392,8 +392,10 @@ void PasswordProtectionService::FillUserPopulation(
LoginReputationClientRequest* request_proto) {
ChromeUserPopulation* user_population = request_proto->mutable_population();
user_population->set_user_population(
IsExtendedReporting() ? ChromeUserPopulation::EXTENDED_REPORTING
: ChromeUserPopulation::SAFE_BROWSING);
IsEnhancedProtection()
? ChromeUserPopulation::ENHANCED_PROTECTION
: IsExtendedReporting() ? ChromeUserPopulation::EXTENDED_REPORTING
: ChromeUserPopulation::SAFE_BROWSING);
user_population->set_profile_management_status(
GetProfileManagementStatus(GetBrowserPolicyConnector()));
user_population->set_is_history_sync_enabled(IsHistorySyncEnabled());
......
......@@ -338,6 +338,8 @@ class PasswordProtectionService : public history::HistoryServiceObserver {
virtual bool IsExtendedReporting() = 0;
virtual bool IsEnhancedProtection() = 0;
virtual bool IsIncognito() = 0;
virtual bool IsPingingEnabled(
......
......@@ -238,7 +238,8 @@ class MockPasswordProtectionNavigationThrottle
DISALLOW_COPY_AND_ASSIGN(MockPasswordProtectionNavigationThrottle);
};
class PasswordProtectionServiceTest : public ::testing::TestWithParam<bool> {
class PasswordProtectionServiceTest
: public ::testing::TestWithParam<testing::tuple<bool, bool>> {
public:
PasswordProtectionServiceTest()
: task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
......@@ -268,7 +269,9 @@ class PasswordProtectionServiceTest : public ::testing::TestWithParam<bool> {
&test_url_loader_factory_),
content_setting_map_);
EXPECT_CALL(*password_protection_service_, IsExtendedReporting())
.WillRepeatedly(Return(GetParam()));
.WillRepeatedly(Return(testing::get<0>(GetParam())));
EXPECT_CALL(*password_protection_service_, IsEnhancedProtection())
.WillRepeatedly(Return(testing::get<1>(GetParam())));
EXPECT_CALL(*password_protection_service_, IsIncognito())
.WillRepeatedly(Return(false));
EXPECT_CALL(*password_protection_service_,
......@@ -378,7 +381,8 @@ class PasswordProtectionServiceTest : public ::testing::TestWithParam<bool> {
void VerifyContentAreaSizeCollection(
const LoginReputationClientRequest& request) {
bool should_report_content_size =
password_protection_service_->IsExtendedReporting() &&
(password_protection_service_->IsExtendedReporting() ||
password_protection_service_->IsEnhancedProtection()) &&
!password_protection_service_->IsIncognito();
EXPECT_EQ(should_report_content_size, request.has_content_area_height());
EXPECT_EQ(should_report_content_size, request.has_content_area_width());
......@@ -1119,7 +1123,8 @@ TEST_P(PasswordProtectionServiceTest,
const auto& reuse_event = actual_request->password_reuse_event();
EXPECT_FALSE(reuse_event.is_chrome_signin_password());
if (password_protection_service_->IsExtendedReporting() &&
if ((password_protection_service_->IsExtendedReporting() ||
password_protection_service_->IsEnhancedProtection()) &&
!password_protection_service_->IsIncognito()) {
ASSERT_EQ(2, reuse_event.domains_matching_password_size());
EXPECT_EQ(kSavedDomain, reuse_event.domains_matching_password(0));
......@@ -1400,8 +1405,9 @@ TEST_P(PasswordProtectionServiceTest,
LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE, true);
base::RunLoop().RunUntilIdle();
bool is_sber = GetParam();
if (is_sber) {
bool is_sber = testing::get<0>(GetParam());
bool is_enhanced_protection = testing::get<1>(GetParam());
if (is_sber || is_enhanced_protection) {
password_protection_service_->WaitForResponse();
ASSERT_NE(nullptr, password_protection_service_->GetLatestRequestProto());
EXPECT_TRUE(password_protection_service_->GetLatestRequestProto()
......@@ -1490,9 +1496,18 @@ TEST_P(PasswordProtectionServiceTest, TestWebContentsDestroyed) {
INSTANTIATE_TEST_SUITE_P(Regular,
PasswordProtectionServiceTest,
::testing::Values(false));
testing::Combine(testing::Values(false),
testing::Values(false)));
INSTANTIATE_TEST_SUITE_P(SBER,
PasswordProtectionServiceTest,
::testing::Values(true));
testing::Combine(testing::Values(true),
testing::Values(false)));
INSTANTIATE_TEST_SUITE_P(ENHANCED_PROTECTION,
PasswordProtectionServiceTest,
testing::Combine(testing::Values(false),
testing::Values(true)));
INSTANTIATE_TEST_SUITE_P(SBER_ENHANCED_PROTECTION,
PasswordProtectionServiceTest,
testing::Combine(testing::Values(true),
testing::Values(true)));
} // namespace safe_browsing
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