Commit 4ee6e77f authored by Dominique Fauteux-Chapleau's avatar Dominique Fauteux-Chapleau Committed by Chromium LUCI CQ

Add profile DM token reading code to ConnectorsService

This adds the main code to read the DM token from the profile instead of
CBCM. The PerProfileConnectorsEnabled feature guards these changes from
affecting working Connector policies. This change is insufficient to
test per-profile Connectors since the authentication and reporting code
still assume the DM token is from the browser.

This adds a tweak to Connector policy test setters to ensure existing
tests don't break and still use the browser DM token.

Bug: 1148789, 1155585
Change-Id: I64bb1714667b0b61fdfdbadffbc153328e22f741
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2590429Reviewed-by: default avatarJulian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: default avatarDaniel Rubery <drubery@chromium.org>
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838139}
parent 02f4cd0d
......@@ -498,6 +498,8 @@ static_library("browser") {
"enterprise/browser_management/browser_management_service.h",
"enterprise/browser_management/browser_management_status_provider.cc",
"enterprise/browser_management/browser_management_status_provider.h",
"enterprise/util/affiliation.cc",
"enterprise/util/affiliation.h",
"enterprise/util/managed_browser_utils.cc",
"enterprise/util/managed_browser_utils.h",
"expired_flags_list.h",
......
......@@ -15,8 +15,10 @@ AnalysisSettings& AnalysisSettings::operator=(AnalysisSettings&&) = default;
AnalysisSettings::~AnalysisSettings() = default;
ReportingSettings::ReportingSettings() = default;
ReportingSettings::ReportingSettings(GURL url, const std::string& dm_token)
: reporting_url(url), dm_token(dm_token) {}
ReportingSettings::ReportingSettings(GURL url,
const std::string& dm_token,
bool per_profile)
: reporting_url(url), dm_token(dm_token), per_profile(per_profile) {}
ReportingSettings::ReportingSettings(ReportingSettings&&) = default;
ReportingSettings& ReportingSettings::operator=(ReportingSettings&&) = default;
ReportingSettings::~ReportingSettings() = default;
......
......@@ -66,13 +66,19 @@ struct AnalysisSettings {
struct ReportingSettings {
ReportingSettings();
explicit ReportingSettings(GURL url, const std::string& dm_token);
explicit ReportingSettings(GURL url,
const std::string& dm_token,
bool per_profile);
ReportingSettings(ReportingSettings&&);
ReportingSettings& operator=(ReportingSettings&&);
~ReportingSettings();
GURL reporting_url;
std::string dm_token;
// Indicates if the report should be made for the profile, or the browser if
// false.
bool per_profile = false;
};
// Returns the pref path corresponding to a connector.
......
......@@ -7,13 +7,20 @@
#include "base/memory/singleton.h"
#include "base/no_destructor.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/enterprise/connectors/common.h"
#include "chrome/browser/enterprise/connectors/connectors_manager.h"
#include "chrome/browser/enterprise/connectors/service_provider_config.h"
#include "chrome/browser/enterprise/util/affiliation.h"
#include "chrome/browser/policy/chrome_browser_policy_connector.h"
#include "chrome/browser/policy/dm_token_utils.h"
#include "chrome/browser/profiles/profile.h"
#include "components/enterprise/browser/controller/browser_dm_token_storage.h"
#include "components/enterprise/common/proto/connectors.pb.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/policy/core/common/cloud/dm_token.h"
#include "components/policy/core/common/cloud/machine_level_user_cloud_policy_manager.h"
#include "components/policy/core/common/cloud/user_cloud_policy_manager.h"
#include "components/policy/core/common/policy_types.h"
#include "components/user_prefs/user_prefs.h"
#include "content/public/browser/browser_context.h"
......@@ -23,6 +30,9 @@ namespace enterprise_connectors {
const base::Feature kEnterpriseConnectorsEnabled{
"EnterpriseConnectorsEnabled", base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kPerProfileConnectorsEnabled{
"PerProfileConnectorsEnabled", base::FEATURE_DISABLED_BY_DEFAULT};
const char kServiceProviderConfig[] = R"({
"version": "1",
"service_providers" : [
......@@ -117,7 +127,7 @@ base::Optional<ReportingSettings> ConnectorsService::GetReportingSettings(
if (!ConnectorsEnabled())
return base::nullopt;
base::Optional<DmToken> dm_token = GetDmToken(ConnectorPref(connector));
base::Optional<DmToken> dm_token = GetDmToken(ConnectorScopePref(connector));
if (!dm_token.has_value())
return base::nullopt;
......@@ -125,6 +135,8 @@ base::Optional<ReportingSettings> ConnectorsService::GetReportingSettings(
connectors_manager_->GetReportingSettings(connector);
if (settings.has_value()) {
settings.value().dm_token = dm_token.value().value;
settings.value().per_profile =
dm_token.value().scope == policy::POLICY_SCOPE_USER;
}
return settings;
......@@ -136,7 +148,7 @@ base::Optional<AnalysisSettings> ConnectorsService::GetAnalysisSettings(
if (!ConnectorsEnabled())
return base::nullopt;
base::Optional<DmToken> dm_token = GetDmToken(ConnectorPref(connector));
base::Optional<DmToken> dm_token = GetDmToken(ConnectorScopePref(connector));
if (!dm_token.has_value())
return base::nullopt;
......@@ -183,10 +195,20 @@ ConnectorsService::DmToken& ConnectorsService::DmToken::operator=(DmToken&&) =
ConnectorsService::DmToken::~DmToken() = default;
base::Optional<ConnectorsService::DmToken> ConnectorsService::GetDmToken(
const char* pref) {
// TODO(crbug.com/1148789): Add code to check the scope of |pref| and handle
// the "user" case.
const char* scope_pref) const {
#if defined(OS_CHROMEOS)
// On CrOS, the device must be affiliated to use the DM token for
// scanning/reporting so we always use the browser DM token.
return GetBrowserDmToken();
#else
return GetPolicyScope(scope_pref) == policy::POLICY_SCOPE_USER
? GetProfileDmToken()
: GetBrowserDmToken();
#endif
}
base::Optional<ConnectorsService::DmToken>
ConnectorsService::GetBrowserDmToken() const {
policy::DMToken dm_token =
policy::GetDMToken(Profile::FromBrowserContext(context_));
......@@ -196,6 +218,60 @@ base::Optional<ConnectorsService::DmToken> ConnectorsService::GetDmToken(
return DmToken(dm_token.value(), policy::POLICY_SCOPE_MACHINE);
}
#if !defined(OS_CHROMEOS)
base::Optional<ConnectorsService::DmToken>
ConnectorsService::GetProfileDmToken() const {
if (!base::FeatureList::IsEnabled(kPerProfileConnectorsEnabled))
return base::nullopt;
if (!CanUseProfileDmToken())
return base::nullopt;
policy::UserCloudPolicyManager* policy_manager =
Profile::FromBrowserContext(context_)->GetUserCloudPolicyManager();
if (!policy_manager || !policy_manager->IsClientRegistered())
return base::nullopt;
return DmToken(policy_manager->core()->client()->dm_token(),
policy::POLICY_SCOPE_USER);
}
bool ConnectorsService::CanUseProfileDmToken() const {
// If the browser isn't managed by CBCM, then the profile DM token can be
// used.
if (!policy::BrowserDMTokenStorage::Get()->RetrieveDMToken().is_valid())
return true;
policy::UserCloudPolicyManager* profile_policy_manager =
Profile::FromBrowserContext(context_)->GetUserCloudPolicyManager();
policy::MachineLevelUserCloudPolicyManager* browser_policy_manager =
g_browser_process->browser_policy_connector()
->machine_level_user_cloud_policy_manager();
if (!profile_policy_manager || !browser_policy_manager ||
!profile_policy_manager->IsClientRegistered() ||
!browser_policy_manager->IsClientRegistered()) {
return false;
}
auto* profile_policy = profile_policy_manager->core()->store()->policy();
auto* browser_policy = browser_policy_manager->core()->store()->policy();
if (!profile_policy || !browser_policy)
return false;
return chrome::enterprise_util::IsProfileAffiliated(*profile_policy,
*browser_policy);
}
#endif // !defined(OS_CHROMEOS)
policy::PolicyScope ConnectorsService::GetPolicyScope(
const char* scope_pref) const {
return static_cast<policy::PolicyScope>(
Profile::FromBrowserContext(context_)->GetPrefs()->GetInteger(
scope_pref));
}
bool ConnectorsService::ConnectorsEnabled() const {
if (!base::FeatureList::IsEnabled(kEnterpriseConnectorsEnabled))
return false;
......
......@@ -25,6 +25,9 @@ namespace enterprise_connectors {
// ConnectorsManager.
extern const base::Feature kEnterpriseConnectorsEnabled;
// Controls whether per-profile Enterprise Connector policies are applied.
extern const base::Feature kPerProfileConnectorsEnabled;
// For the moment, service provider configurations are static and only support
// google endpoints. Therefore the configuration is placed here directly.
// Once the configuration becomes more dynamic this static string will be
......@@ -71,8 +74,26 @@ class ConnectorsService : public KeyedService {
// policy used to get a DM token.
policy::PolicyScope scope;
};
base::Optional<DmToken> GetDmToken(const char* pref);
// Returns the DM token to use with the given |scope_pref|. That pref should
// contain either POLICY_SCOPE_MACHINE or POLICY_SCOPE_USER.
base::Optional<DmToken> GetDmToken(const char* scope_pref) const;
base::Optional<DmToken> GetBrowserDmToken() const;
#if !defined(OS_CHROMEOS)
base::Optional<DmToken> GetProfileDmToken() const;
// Returns true if the browser isn't managed by CBCM, otherwise this checks if
// the affiliations IDs from the profile and browser policy fetching responses
// indicate that the same customer manages both.
bool CanUseProfileDmToken() const;
#endif
// Returns the policy::PolicyScope stored in the given |scope_pref|.
policy::PolicyScope GetPolicyScope(const char* scope_pref) const;
// Returns whether Connectors are enabled at all. This can be false if:
// - The kEnterpriseConnectorsEnabled feature is disabled
// - The profile is incognito
bool ConnectorsEnabled() const;
content::BrowserContext* context_;
......
......@@ -11,6 +11,7 @@
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile_manager.h"
#include "components/enterprise/common/proto/connectors.pb.h"
#include "components/policy/core/common/policy_types.h"
#include "content/public/test/browser_task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
......@@ -130,6 +131,8 @@ class ConnectorsServiceReportingFeatureTest
const char* pref() const { return ConnectorPref(connector()); }
const char* scope_pref() const { return ConnectorScopePref(connector()); }
const char* pref_value() const {
switch (policy_value()) {
case 1:
......@@ -155,8 +158,11 @@ class ConnectorsServiceReportingFeatureTest
};
TEST_P(ConnectorsServiceReportingFeatureTest, Test) {
if (policy_value() != 0)
if (policy_value() != 0) {
profile_->GetPrefs()->Set(pref(), *base::JSONReader::Read(pref_value()));
profile_->GetPrefs()->SetInteger(scope_pref(),
policy::POLICY_SCOPE_MACHINE);
}
auto settings = ConnectorsServiceFactory::GetForBrowserContext(profile_)
->GetReportingSettings(connector());
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/enterprise/util/affiliation.h"
#include <set>
namespace chrome {
namespace enterprise_util {
bool IsProfileAffiliated(
const enterprise_management::PolicyData& profile_policy,
const enterprise_management::PolicyData& browser_policy) {
std::set<std::string> profile_affiliation_ids;
profile_affiliation_ids.insert(profile_policy.user_affiliation_ids().begin(),
profile_policy.user_affiliation_ids().end());
for (const std::string& browser_affiliation_id :
browser_policy.device_affiliation_ids()) {
if (profile_affiliation_ids.count(browser_affiliation_id))
return true;
}
return false;
}
} // namespace enterprise_util
} // namespace chrome
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_ENTERPRISE_UTIL_AFFILIATION_H_
#define CHROME_BROWSER_ENTERPRISE_UTIL_AFFILIATION_H_
#include "device_management_backend.pb.h"
namespace chrome {
namespace enterprise_util {
// Returns true if the profile and browser are managed by the same customer
// (affiliated). This is determined by comparing affiliation IDs obtained in the
// policy fetching response. If either policies has no affiliation IDs, this
// function returns false.
bool IsProfileAffiliated(
const enterprise_management::PolicyData& profile_policy,
const enterprise_management::PolicyData& browser_policy);
} // namespace enterprise_util
} // namespace chrome
#endif // CHROME_BROWSER_ENTERPRISE_UTIL_AFFILIATION_H_
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/enterprise/util/affiliation.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace chrome {
namespace enterprise_util {
namespace {
const char kAffiliationID1[] = "id1";
const char kAffiliationID2[] = "id2";
} // namespace
TEST(BrowserAffiliationTest, Affiliated) {
enterprise_management::PolicyData profile_policy;
profile_policy.add_user_affiliation_ids(kAffiliationID1);
enterprise_management::PolicyData browser_policy;
browser_policy.add_device_affiliation_ids(kAffiliationID1);
EXPECT_TRUE(IsProfileAffiliated(profile_policy, browser_policy));
}
TEST(BrowserAffiliationTest, Unaffiliated) {
enterprise_management::PolicyData profile_policy;
profile_policy.add_user_affiliation_ids(kAffiliationID1);
enterprise_management::PolicyData browser_policy;
browser_policy.add_device_affiliation_ids(kAffiliationID2);
EXPECT_FALSE(IsProfileAffiliated(profile_policy, browser_policy));
}
TEST(BrowserAffiliationTest, BrowserPolicyEmpty) {
enterprise_management::PolicyData profile_policy;
profile_policy.add_user_affiliation_ids(kAffiliationID1);
enterprise_management::PolicyData browser_policy;
EXPECT_FALSE(IsProfileAffiliated(profile_policy, browser_policy));
}
TEST(BrowserAffiliationTest, ProfilePolicyEmpty) {
enterprise_management::PolicyData profile_policy;
enterprise_management::PolicyData browser_policy;
browser_policy.add_device_affiliation_ids(kAffiliationID2);
EXPECT_FALSE(IsProfileAffiliated(profile_policy, browser_policy));
}
} // namespace enterprise_util
} // namespace chrome
......@@ -49,8 +49,10 @@ class DeepScanningBrowserTestBase : public InProcessBrowserTest {
const std::vector<base::FilePath>& created_file_paths() const;
private:
protected:
base::test::ScopedFeatureList scoped_feature_list_;
private:
base::RepeatingClosure quit_closure_;
enterprise_connectors::ContentAnalysisResponse
connector_status_callback_response_;
......
......@@ -13,6 +13,7 @@
#include "chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router.h"
#include "components/policy/core/common/cloud/mock_cloud_policy_client.h"
#include "components/policy/core/common/cloud/realtime_reporting_job_configuration.h"
#include "components/policy/core/common/policy_types.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
#include "content/public/browser/browser_context.h"
......@@ -343,16 +344,22 @@ void EventReportValidator::SetDoneClosure(base::RepeatingClosure closure) {
void SetAnalysisConnector(PrefService* prefs,
enterprise_connectors::AnalysisConnector connector,
const std::string& pref_value) {
const std::string& pref_value,
bool machine_scope) {
ListPrefUpdate settings_list(prefs, ConnectorPref(connector));
DCHECK(settings_list.Get());
if (!settings_list->empty())
settings_list->Clear();
settings_list->Append(*base::JSONReader::Read(pref_value));
prefs->SetInteger(
ConnectorScopePref(connector),
machine_scope ? policy::POLICY_SCOPE_MACHINE : policy::POLICY_SCOPE_USER);
}
void SetOnSecurityEventReporting(PrefService* prefs, bool enabled) {
void SetOnSecurityEventReporting(PrefService* prefs,
bool enabled,
bool machine_scope) {
ListPrefUpdate settings_list(prefs,
enterprise_connectors::kOnSecurityEventPref);
DCHECK(settings_list.Get());
......@@ -364,18 +371,22 @@ void SetOnSecurityEventReporting(PrefService* prefs, bool enabled) {
base::Value("google"));
settings_list->Append(std::move(settings));
}
prefs->SetInteger(enterprise_connectors::kOnSecurityEventScopePref,
machine_scope ? policy::POLICY_SCOPE_MACHINE
: policy::POLICY_SCOPE_USER);
} else {
settings_list->ClearList();
prefs->ClearPref(enterprise_connectors::kOnSecurityEventScopePref);
}
}
void ClearAnalysisConnector(
PrefService* prefs,
enterprise_connectors::AnalysisConnector connector) {
ListPrefUpdate settings_list(prefs, ConnectorPref(connector));
DCHECK(settings_list.Get());
settings_list->Clear();
prefs->ClearPref(ConnectorScopePref(connector));
}
} // namespace safe_browsing
......@@ -148,8 +148,11 @@ class EventReportValidator {
// Helper functions that set Connector policies for testing.
void SetAnalysisConnector(PrefService* prefs,
enterprise_connectors::AnalysisConnector connector,
const std::string& pref_value);
void SetOnSecurityEventReporting(PrefService* prefs, bool enabled);
const std::string& pref_value,
bool machine_scope = true);
void SetOnSecurityEventReporting(PrefService* prefs,
bool enabled,
bool machine_scope = true);
void ClearAnalysisConnector(PrefService* prefs,
enterprise_connectors::AnalysisConnector connector);
......
......@@ -1041,6 +1041,7 @@ if (!is_android) {
"../browser/download/download_frame_policy_browsertest.cc",
"../browser/download/download_started_animation_browsertest.cc",
"../browser/download/save_page_browsertest.cc",
"../browser/enterprise/connectors/connectors_service_browsertest.cc",
"../browser/enterprise/connectors/content_analysis_delegate_browsertest.cc",
"../browser/enterprise/connectors/content_analysis_dialog_browsertest.cc",
"../browser/enterprise/reporting/report_scheduler_browsertest.cc",
......@@ -3489,6 +3490,7 @@ test("unit_tests") {
"../browser/engagement/site_engagement_helper_unittest.cc",
"../browser/engagement/site_engagement_score_unittest.cc",
"../browser/engagement/site_engagement_service_unittest.cc",
"../browser/enterprise/util/affiliation_unittest.cc",
"../browser/enterprise/util/managed_browser_utils_unittest.cc",
"../browser/external_protocol/external_protocol_handler_unittest.cc",
"../browser/federated_learning/floc_id_provider_unittest.cc",
......
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