Skip to content

Commit

Permalink
[M110][URLFiltering] Include access token in the call to ChromeEnterp…
Browse files Browse the repository at this point in the history 
…riseRealTimeUrlLookup Service

(cherry picked from commit d6206e4)

Bug: 1402746
Change-Id: Ib091b62b916219e1e47ec8b683e7784e08d1a975
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4113106
Reviewed-by: Xinghui Lu <xinghuilu@chromium.org>
Commit-Queue: Sneha Nagpaul <snehanagpaul@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1085620}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4133368
Cr-Commit-Position: refs/branch-heads/5481@{#134}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Sneha Nagpaul authored and Chromium LUCI CQ committed Jan 5, 2023
1 parent 6bd8e7e commit c483f9f
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 7 deletions.
1 change: 1 addition & 0 deletions chrome/browser/safe_browsing/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ static_library("safe_browsing") {
"//components/safe_browsing/content/browser/triggers:trigger_throttler",
"//components/safe_browsing/content/common:file_type_policies",
"//components/safe_browsing/core/browser",
"//components/safe_browsing/core/browser:token_fetcher",
"//components/safe_browsing/core/browser:verdict_cache_manager",
"//components/safe_browsing/core/browser/db:allowlist_checker_client",
"//components/safe_browsing/core/browser/db:metadata_proto",
Expand Down
35 changes: 32 additions & 3 deletions chrome/browser/safe_browsing/chrome_enterprise_url_lookup_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@
#include "chrome/browser/enterprise/connectors/connectors_service.h"
#include "chrome/browser/policy/dm_token_utils.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "components/policy/core/common/cloud/dm_token.h"
#include "components/prefs/pref_service.h"
#include "components/safe_browsing/core/browser/realtime/policy_engine.h"
#include "components/safe_browsing/core/browser/realtime/url_lookup_service_base.h"
#include "components/safe_browsing/core/browser/referrer_chain_provider.h"
#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h"
#include "components/safe_browsing/core/browser/sync/sync_utils.h"
#include "components/safe_browsing/core/browser/verdict_cache_manager.h"
#include "components/safe_browsing/core/common/features.h"
#include "components/safe_browsing/core/common/proto/csd.pb.h"
Expand All @@ -30,14 +33,16 @@ ChromeEnterpriseRealTimeUrlLookupService::
Profile* profile,
base::RepeatingCallback<ChromeUserPopulation()>
get_user_population_callback,
std::unique_ptr<SafeBrowsingTokenFetcher> token_fetcher,
enterprise_connectors::ConnectorsService* connectors_service,
ReferrerChainProvider* referrer_chain_provider)
: RealTimeUrlLookupServiceBase(url_loader_factory,
cache_manager,
get_user_population_callback,
referrer_chain_provider),
profile_(profile),
connectors_service_(connectors_service) {}
connectors_service_(connectors_service),
token_fetcher_(std::move(token_fetcher)) {}

ChromeEnterpriseRealTimeUrlLookupService::
~ChromeEnterpriseRealTimeUrlLookupService() = default;
Expand All @@ -51,7 +56,11 @@ bool ChromeEnterpriseRealTimeUrlLookupService::CanPerformFullURLLookup() const {

bool ChromeEnterpriseRealTimeUrlLookupService::
CanPerformFullURLLookupWithToken() const {
// URL lookup with token is disabled for enterprise users.
DCHECK(CanPerformFullURLLookup());
if (safe_browsing::SyncUtils::IsPrimaryAccountSignedIn(
IdentityManagerFactory::GetForProfile(profile_))) {
return base::FeatureList::IsEnabled((kRealTimeUrlFilteringForEnterprise));
}
return false;
}

Expand Down Expand Up @@ -94,7 +103,27 @@ void ChromeEnterpriseRealTimeUrlLookupService::GetAccessToken(
RTLookupRequestCallback request_callback,
RTLookupResponseCallback response_callback,
scoped_refptr<base::SequencedTaskRunner> callback_task_runner) {
NOTREACHED() << "URL lookup with token is disabled for enterprise users.";
DCHECK(base::FeatureList::IsEnabled((kRealTimeUrlFilteringForEnterprise)));
token_fetcher_->Start(base::BindOnce(
&ChromeEnterpriseRealTimeUrlLookupService::OnGetAccessToken,
weak_factory_.GetWeakPtr(), url, last_committed_url, is_mainframe,
std::move(request_callback), std::move(response_callback),
std::move(callback_task_runner), base::TimeTicks::Now()));
}

void ChromeEnterpriseRealTimeUrlLookupService::OnGetAccessToken(
const GURL& url,
const GURL& last_committed_url,
bool is_mainframe,
RTLookupRequestCallback request_callback,
RTLookupResponseCallback response_callback,
scoped_refptr<base::SequencedTaskRunner> callback_task_runner,
base::TimeTicks get_token_start_time,
const std::string& access_token) {
SendRequest(url, last_committed_url, is_mainframe, access_token,
std::move(request_callback), std::move(response_callback),
std::move(callback_task_runner),
/* is_sampled_report */ false);
}

absl::optional<std::string>
Expand Down
17 changes: 17 additions & 0 deletions chrome/browser/safe_browsing/chrome_enterprise_url_lookup_service.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/memory/raw_ptr.h"
#include "chrome/browser/enterprise/connectors/connectors_service.h"
#include "components/safe_browsing/core/browser/realtime/url_lookup_service_base.h"
#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h"
#include "components/safe_browsing/core/common/proto/csd.pb.h"
#include "url/gurl.h"

Expand Down Expand Up @@ -40,6 +41,7 @@ class ChromeEnterpriseRealTimeUrlLookupService
Profile* profile,
base::RepeatingCallback<ChromeUserPopulation()>
get_user_population_callback,
std::unique_ptr<SafeBrowsingTokenFetcher> token_fetcher,
enterprise_connectors::ConnectorsService* connectors_service,
ReferrerChainProvider* referrer_chain_provider);

Expand Down Expand Up @@ -72,6 +74,18 @@ class ChromeEnterpriseRealTimeUrlLookupService
RTLookupRequestCallback request_callback,
RTLookupResponseCallback response_callback,
scoped_refptr<base::SequencedTaskRunner> callback_task_runner) override;

// Called when the access token is obtained from |token_fetcher_|.
void OnGetAccessToken(
const GURL& url,
const GURL& last_committed_url,
bool is_mainframe,
RTLookupRequestCallback request_callback,
RTLookupResponseCallback response_callback,
scoped_refptr<base::SequencedTaskRunner> callback_task_runner,
base::TimeTicks get_token_start_time,
const std::string& access_token);

absl::optional<std::string> GetDMTokenString() const override;
bool ShouldIncludeCredentials() const override;
double GetMinAllowedTimestampForReferrerChains() const override;
Expand All @@ -82,6 +96,9 @@ class ChromeEnterpriseRealTimeUrlLookupService
// Unowned pointer to ConnectorsService, used to get a DM token.
raw_ptr<enterprise_connectors::ConnectorsService> connectors_service_;

// The token fetcher used for getting access token.
std::unique_ptr<SafeBrowsingTokenFetcher> token_fetcher_;

friend class ChromeEnterpriseRealTimeUrlLookupServiceTest;

base::WeakPtrFactory<ChromeEnterpriseRealTimeUrlLookupService> weak_factory_{
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/safe_browsing/chrome_enterprise_url_lookup_service_factory.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
#include "chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager_factory.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "chrome/browser/safe_browsing/verdict_cache_manager_factory.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "components/safe_browsing/content/browser/safe_browsing_navigation_observer_manager.h"
#include "components/safe_browsing/core/browser/sync/safe_browsing_primary_account_token_fetcher.h"
#include "components/safe_browsing/core/browser/sync/sync_utils.h"
#include "components/safe_browsing/core/browser/verdict_cache_manager.h"
#include "components/safe_browsing/core/common/features.h"
Expand Down Expand Up @@ -62,6 +64,8 @@ ChromeEnterpriseRealTimeUrlLookupServiceFactory::BuildServiceInstanceFor(
network::SharedURLLoaderFactory::Create(std::move(url_loader_factory)),
VerdictCacheManagerFactory::GetForProfile(profile), profile,
base::BindRepeating(&safe_browsing::GetUserPopulationForProfile, profile),
std::make_unique<SafeBrowsingPrimaryAccountTokenFetcher>(
IdentityManagerFactory::GetForProfile(profile)),
enterprise_connectors::ConnectorsServiceFactory::GetForBrowserContext(
profile),
SafeBrowsingNavigationObserverManagerFactory::GetForBrowserContext(
Expand Down
75 changes: 71 additions & 4 deletions chrome/browser/safe_browsing/chrome_enterprise_url_lookup_service_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,20 @@
#include "chrome/browser/enterprise/connectors/connectors_service.h"
#include "chrome/browser/policy/dm_token_utils.h"
#include "chrome/browser/safe_browsing/chrome_user_population_helper.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/test/base/testing_profile.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/policy/core/common/cloud/dm_token.h"
#include "components/policy/core/common/policy_types.h"
#include "components/safe_browsing/core/browser/referrer_chain_provider.h"
#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h"
#include "components/safe_browsing/core/browser/sync/sync_utils.h"
#include "components/safe_browsing/core/browser/test_safe_browsing_token_fetcher.h"
#include "components/safe_browsing/core/browser/verdict_cache_manager.h"
#include "components/safe_browsing/core/common/features.h"
#include "components/safe_browsing/core/common/proto/csd.pb.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
#include "components/signin/public/identity_manager/identity_test_utils.h"
#include "components/sync/test/test_sync_service.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "content/public/browser/browser_task_traits.h"
Expand Down Expand Up @@ -89,6 +93,14 @@ class ChromeEnterpriseRealTimeUrlLookupServiceTest : public PlatformTest {
TestingProfile::Builder builder;
test_profile_ = builder.Build();

signin::IdentityManager* identity_manager =
IdentityManagerFactory::GetForProfile(test_profile_.get());
signin::SetPrimaryAccount(identity_manager, "test@example.com",
signin::ConsentLevel::kSignin);

auto token_fetcher = std::make_unique<TestSafeBrowsingTokenFetcher>();
raw_token_fetcher_ = token_fetcher.get();

enterprise_rt_service_ = std::make_unique<
ChromeEnterpriseRealTimeUrlLookupService>(
test_shared_loader_factory_, cache_manager_.get(), test_profile_.get(),
Expand All @@ -105,6 +117,7 @@ class ChromeEnterpriseRealTimeUrlLookupServiceTest : public PlatformTest {
return population;
},
test_profile_.get(), &test_sync_service_),
std::move(token_fetcher),
enterprise_connectors::ConnectorsServiceFactory::GetForBrowserContext(
test_profile_.get()),
referrer_chain_provider_.get());
Expand All @@ -127,6 +140,18 @@ class ChromeEnterpriseRealTimeUrlLookupServiceTest : public PlatformTest {
return enterprise_rt_service_->GetCachedRealTimeUrlVerdict(url);
}

ChromeEnterpriseRealTimeUrlLookupService* enterprise_rt_service() {
return enterprise_rt_service_.get();
}

void FulfillAccessTokenRequest(std::string token) {
raw_token_fetcher_->RunAccessTokenCallback(token);
}

TestSafeBrowsingTokenFetcher* raw_token_fetcher() {
return raw_token_fetcher_;
}

void MayBeCacheRealTimeUrlVerdict(
GURL url,
RTLookupResponse::ThreatInfo::VerdictType verdict_type,
Expand Down Expand Up @@ -168,17 +193,14 @@ class ChromeEnterpriseRealTimeUrlLookupServiceTest : public PlatformTest {
expected_response_str);
}

ChromeEnterpriseRealTimeUrlLookupService* enterprise_rt_service() {
return enterprise_rt_service_.get();
}

network::TestURLLoaderFactory test_url_loader_factory_;
scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
std::unique_ptr<ChromeEnterpriseRealTimeUrlLookupService>
enterprise_rt_service_;
std::unique_ptr<VerdictCacheManager> cache_manager_;
scoped_refptr<HostContentSettingsMap> content_setting_map_;
content::BrowserTaskEnvironment task_environment_;
raw_ptr<TestSafeBrowsingTokenFetcher> raw_token_fetcher_ = nullptr;
sync_preferences::TestingPrefServiceSyncable test_pref_service_;
std::unique_ptr<TestingProfile> test_profile_;
syncer::TestSyncService test_sync_service_;
Expand Down Expand Up @@ -250,6 +272,51 @@ TEST_F(ChromeEnterpriseRealTimeUrlLookupServiceTest,
EXPECT_NE(nullptr, GetCachedRealTimeUrlVerdict(url));
}

TEST_F(ChromeEnterpriseRealTimeUrlLookupServiceTest,
TestStartLookup_RequestWithDmTokenAndAccessToken) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(kRealTimeUrlFilteringForEnterprise);
GURL url("http://example.test/");
SetUpRTLookupResponse(RTLookupResponse::ThreatInfo::DANGEROUS,
RTLookupResponse::ThreatInfo::SOCIAL_ENGINEERING, 60,
"example.test/",
RTLookupResponse::ThreatInfo::COVERING_MATCH);
SetDMTokenForTesting(policy::DMToken::CreateValidTokenForTesting("dm_token"));
ReferrerChain returned_referrer_chain;
EXPECT_CALL(*referrer_chain_provider_,
IdentifyReferrerChainByPendingEventURL(_, _, _))
.WillOnce(DoAll(SetArgPointee<2>(returned_referrer_chain),
Return(ReferrerChainProvider::SUCCESS)));

base::MockCallback<RTLookupResponseCallback> response_callback;
enterprise_rt_service()->StartLookup(
url, last_committed_url_, is_mainframe_,
base::BindOnce(
[](std::unique_ptr<RTLookupRequest> request, std::string token) {
EXPECT_EQ("http://example.test/", request->url());
EXPECT_EQ("dm_token", request->dm_token());
EXPECT_EQ(ChromeUserPopulation::SAFE_BROWSING,
request->population().user_population());
EXPECT_TRUE(request->population().is_history_sync_enabled());
EXPECT_EQ(ChromeUserPopulation::NOT_MANAGED,
request->population().profile_management_status());
EXPECT_TRUE(request->population().is_under_advanced_protection());
EXPECT_EQ("access_token_string", token);
}),
response_callback.Get(), content::GetIOThreadTaskRunner({}));

EXPECT_CALL(response_callback, Run(/* is_rt_lookup_successful */ true,
/* is_cached_response */ false, _));

EXPECT_TRUE(raw_token_fetcher()->WasStartCalled());
FulfillAccessTokenRequest("access_token_string");

task_environment_.RunUntilIdle();

// Check the response is cached.
EXPECT_NE(nullptr, GetCachedRealTimeUrlVerdict(url));
}

TEST_F(ChromeEnterpriseRealTimeUrlLookupServiceTest,
TestCanCheckSafeBrowsingHighConfidenceAllowlist_BypassAllowlistFeature) {
base::test::ScopedFeatureList scoped_feature_list;
Expand Down

0 comments on commit c483f9f

Please sign in to comment.