Commit 19646b4d authored by Owen Min's avatar Owen Min Committed by Commit Bot

Component cloud policies can overrides platform ones with policy.

With policy enabled, component cloud policies use POLICY_SOURCE_PRIORITY_CLOUD
instead to override platform policies that have same scope.

Bug: 749530
Change-Id: I7d53034073ebd24034fb3c0dfe7034229d683089
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1489902
Commit-Queue: Owen Min <zmin@chromium.org>
Reviewed-by: default avatarMaksim Ivanov <emaxx@chromium.org>
Reviewed-by: default avatarJulian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638355}
parent 28580fd0
......@@ -42,6 +42,7 @@
#include "components/policy/core/common/cloud/cloud_policy_core.h"
#include "components/policy/core/common/cloud/cloud_policy_service.h"
#include "components/policy/core/common/cloud/cloud_policy_store.h"
#include "components/policy/core/common/policy_types.h"
#include "components/policy/core/common/remote_commands/remote_commands_factory.h"
#include "components/policy/core/common/schema_registry.h"
#include "components/policy/proto/device_management_backend.pb.h"
......@@ -287,7 +288,8 @@ void DeviceCloudPolicyManagerChromeOS::StartConnection(
CHECK(signin_profile_forwarding_schema_registry_);
CreateComponentCloudPolicyService(
dm_protocol::kChromeSigninExtensionPolicyType,
component_policy_cache_dir, client_to_connect.get(),
component_policy_cache_dir, POLICY_SOURCE_CLOUD,
client_to_connect.get(),
signin_profile_forwarding_schema_registry_.get());
}
......
......@@ -39,6 +39,7 @@
#include "components/policy/core/common/cloud/resource_cache.h"
#include "components/policy/core/common/policy_namespace.h"
#include "components/policy/core/common/policy_switches.h"
#include "components/policy/core/common/policy_types.h"
#include "components/policy/policy_constants.h"
#include "components/policy/proto/device_management_backend.pb.h"
#include "content/public/browser/browser_thread.h"
......@@ -255,8 +256,9 @@ void DeviceLocalAccountPolicyBroker::CreateComponentCloudPolicyService(
/* max_cache_size */ base::nullopt));
component_policy_service_.reset(new ComponentCloudPolicyService(
dm_protocol::kChromeExtensionPolicyType, this, &schema_registry_, core(),
client, std::move(resource_cache), resource_cache_task_runner_));
dm_protocol::kChromeExtensionPolicyType, POLICY_SOURCE_CLOUD, this,
&schema_registry_, core(), client, std::move(resource_cache),
resource_cache_task_runner_));
}
DeviceLocalAccountPolicyService::DeviceLocalAccountPolicyService(
......
......@@ -243,7 +243,7 @@ void UserCloudPolicyManagerChromeOS::Connect(
chromeos::GetDeviceDMTokenForUserPolicyGetter(account_id_));
CreateComponentCloudPolicyService(
dm_protocol::kChromeExtensionPolicyType, component_policy_cache_path_,
cloud_policy_client.get(), schema_registry());
POLICY_SOURCE_CLOUD, cloud_policy_client.get(), schema_registry());
core()->Connect(std::move(cloud_policy_client));
client()->AddObserver(this);
......
......@@ -121,6 +121,7 @@ void CloudPolicyManager::GetChromePolicy(PolicyMap* policy_map) {
void CloudPolicyManager::CreateComponentCloudPolicyService(
const std::string& policy_type,
const base::FilePath& policy_cache_path,
PolicySource policy_source,
CloudPolicyClient* client,
SchemaRegistry* schema_registry) {
#if !defined(OS_ANDROID) && !defined(OS_IOS)
......@@ -149,7 +150,7 @@ void CloudPolicyManager::CreateComponentCloudPolicyService(
std::unique_ptr<ResourceCache> resource_cache(new ResourceCache(
policy_cache_path, task_runner, /* max_cache_size */ base::nullopt));
component_policy_service_.reset(new ComponentCloudPolicyService(
policy_type, this, schema_registry, core(), client,
policy_type, policy_source, this, schema_registry, core(), client,
std::move(resource_cache), task_runner));
#endif // !defined(OS_ANDROID) && !defined(OS_IOS)
}
......
......@@ -15,6 +15,7 @@
#include "components/policy/core/common/cloud/cloud_policy_store.h"
#include "components/policy/core/common/cloud/component_cloud_policy_service.h"
#include "components/policy/core/common/configuration_policy_provider.h"
#include "components/policy/core/common/policy_types.h"
#include "components/policy/policy_export.h"
#include "components/prefs/pref_member.h"
#include "services/network/public/cpp/network_connection_tracker.h"
......@@ -79,6 +80,7 @@ class POLICY_EXPORT CloudPolicyManager
void CreateComponentCloudPolicyService(
const std::string& policy_type,
const base::FilePath& policy_cache_path,
PolicySource policy_source,
CloudPolicyClient* client,
SchemaRegistry* schema_registry);
......
......@@ -76,7 +76,8 @@ class ComponentCloudPolicyService::Backend
scoped_refptr<base::SequencedTaskRunner> service_task_runner,
std::unique_ptr<ResourceCache> cache,
std::unique_ptr<ExternalPolicyDataFetcher> external_policy_data_fetcher,
const std::string& policy_type);
const std::string& policy_type,
PolicySource policy_source);
~Backend() override;
......@@ -139,13 +140,14 @@ ComponentCloudPolicyService::Backend::Backend(
scoped_refptr<base::SequencedTaskRunner> service_task_runner,
std::unique_ptr<ResourceCache> cache,
std::unique_ptr<ExternalPolicyDataFetcher> external_policy_data_fetcher,
const std::string& policy_type)
const std::string& policy_type,
PolicySource policy_source)
: service_(service),
task_runner_(task_runner),
service_task_runner_(service_task_runner),
cache_(std::move(cache)),
external_policy_data_fetcher_(std::move(external_policy_data_fetcher)),
store_(this, cache_.get(), policy_type) {
store_(this, cache_.get(), policy_type, policy_source) {
// This class is allowed to be instantiated on any thread.
DETACH_FROM_SEQUENCE(sequence_checker_);
}
......@@ -264,6 +266,7 @@ void ComponentCloudPolicyService::Backend::UpdateWithLastFetchedPolicy() {
ComponentCloudPolicyService::ComponentCloudPolicyService(
const std::string& policy_type,
PolicySource policy_source,
Delegate* delegate,
SchemaRegistry* schema_registry,
CloudPolicyCore* core,
......@@ -290,7 +293,7 @@ ComponentCloudPolicyService::ComponentCloudPolicyService(
base::ThreadTaskRunnerHandle::Get(), std::move(cache),
external_policy_data_fetcher_backend_->CreateFrontend(
backend_task_runner_),
policy_type));
policy_type, policy_source));
// Observe the schema registry for keeping |current_schema_map_| up to date.
schema_registry_->AddObserver(this);
......
......@@ -19,6 +19,7 @@
#include "components/policy/core/common/cloud/cloud_policy_store.h"
#include "components/policy/core/common/policy_bundle.h"
#include "components/policy/core/common/policy_namespace.h"
#include "components/policy/core/common/policy_types.h"
#include "components/policy/core/common/schema_registry.h"
#include "components/policy/policy_export.h"
......@@ -65,6 +66,11 @@ class POLICY_EXPORT ComponentCloudPolicyService
// allowed values are: |dm_protocol::kChromeExtensionPolicyType|,
// |dm_protocol::kChromeSigninExtensionPolicyType|.
//
// |policy_source| specifies where the policy originates from, and can be used
// to configure precedence when the same components are configured by policies
// from different sources. It only accepts POLICY_SOURCE_CLOUD and
// POLICY_SOURCE_PRIORITY_CLOUD now.
//
// The |delegate| is notified of updates to the downloaded policies and must
// outlive this object.
//
......@@ -89,6 +95,7 @@ class POLICY_EXPORT ComponentCloudPolicyService
// |backend_task_runner|, which must support file I/O.
ComponentCloudPolicyService(
const std::string& policy_type,
PolicySource policy_source,
Delegate* delegate,
SchemaRegistry* schema_registry,
CloudPolicyCore* core,
......
......@@ -12,6 +12,7 @@ ComponentCloudPolicyService::Delegate::~Delegate() {}
ComponentCloudPolicyService::ComponentCloudPolicyService(
const std::string& policy_type,
PolicySource policy_source,
Delegate* delegate,
SchemaRegistry* schema_registry,
CloudPolicyCore* core,
......
......@@ -142,8 +142,9 @@ class ComponentCloudPolicyServiceTest : public testing::Test {
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&loader_factory_));
service_.reset(new ComponentCloudPolicyService(
dm_protocol::kChromeExtensionPolicyType, &delegate_, &registry_, &core_,
client_, std::move(owned_cache_), base::ThreadTaskRunnerHandle::Get()));
dm_protocol::kChromeExtensionPolicyType, POLICY_SOURCE_CLOUD,
&delegate_, &registry_, &core_, client_, std::move(owned_cache_),
base::ThreadTaskRunnerHandle::Get()));
client_->SetDMToken(ComponentCloudPolicyBuilder::kFakeToken);
EXPECT_EQ(1u, client_->types_to_fetch_.size());
......
......@@ -22,7 +22,6 @@
#include "components/policy/core/common/cloud/resource_cache.h"
#include "components/policy/core/common/external_data_fetcher.h"
#include "components/policy/core/common/policy_map.h"
#include "components/policy/core/common/policy_types.h"
#include "components/policy/proto/chrome_extension_policy.pb.h"
#include "components/policy/proto/device_management_backend.pb.h"
#include "crypto/sha2.h"
......@@ -87,14 +86,18 @@ ComponentCloudPolicyStore::Delegate::~Delegate() {}
ComponentCloudPolicyStore::ComponentCloudPolicyStore(
Delegate* delegate,
ResourceCache* cache,
const std::string& policy_type)
const std::string& policy_type,
PolicySource policy_source)
: delegate_(delegate),
cache_(cache),
domain_constants_(GetDomainConstantsForType(policy_type)) {
domain_constants_(GetDomainConstantsForType(policy_type)),
policy_source_(policy_source) {
// Allow the store to be created on a different thread than the thread that
// will end up using it.
DETACH_FROM_SEQUENCE(sequence_checker_);
DCHECK(domain_constants_);
DCHECK(policy_source == POLICY_SOURCE_CLOUD ||
policy_source == POLICY_SOURCE_PRIORITY_CLOUD);
}
ComponentCloudPolicyStore::~ComponentCloudPolicyStore() {
......@@ -433,7 +436,7 @@ bool ComponentCloudPolicyStore::ParsePolicy(const std::string& data,
level = POLICY_LEVEL_RECOMMENDED;
}
policy->Set(it.key(), level, domain_constants_->scope, POLICY_SOURCE_CLOUD,
policy->Set(it.key(), level, domain_constants_->scope, policy_source_,
std::move(value), nullptr);
}
......
......@@ -16,6 +16,7 @@
#include "components/policy/core/common/cloud/resource_cache.h"
#include "components/policy/core/common/policy_bundle.h"
#include "components/policy/core/common/policy_namespace.h"
#include "components/policy/core/common/policy_types.h"
#include "components/policy/policy_export.h"
namespace enterprise_management {
......@@ -57,9 +58,14 @@ class POLICY_EXPORT ComponentCloudPolicyStore {
// kChromeExtensionPolicyType, kChromeMachineLevelExtensionCloudPolicyType.
// Please update component_cloud_policy_store.cc in case there is new policy
// type added.
// |policy_source| specifies where the policy originates from, and can be used
// to configure precedence when the same components are configured by policies
// from different sources. It only accepts POLICY_SOURCE_CLOUD and
// POLICY_SOURCE_PRIORITY_CLOUD now.
ComponentCloudPolicyStore(Delegate* delegate,
ResourceCache* cache,
const std::string& policy_type);
const std::string& policy_type,
PolicySource policy_source);
~ComponentCloudPolicyStore();
// Helper that returns true for PolicyDomains that can be managed by this
......@@ -162,6 +168,8 @@ class POLICY_EXPORT ComponentCloudPolicyStore {
const DomainConstants* domain_constants_;
const PolicySource policy_source_;
SEQUENCE_CHECKER(sequence_checker_);
DISALLOW_COPY_AND_ASSIGN(ComponentCloudPolicyStore);
......
......@@ -104,12 +104,14 @@ class ComponentCloudPolicyStoreTest : public testing::Test {
PolicyBuilder::kFakePublicKeyVersion);
}
void SetupExpectBundleWithScope(const PolicyScope& scope) {
void SetupExpectBundleWithScope(
const PolicyScope& scope,
const PolicySource& source = POLICY_SOURCE_CLOUD) {
PolicyMap& policy = expected_bundle_.Get(kTestPolicyNS);
policy.Clear();
policy.Set("Name", POLICY_LEVEL_MANDATORY, scope, POLICY_SOURCE_CLOUD,
policy.Set("Name", POLICY_LEVEL_MANDATORY, scope, source,
std::make_unique<base::Value>("disabled"), nullptr);
policy.Set("Second", POLICY_LEVEL_RECOMMENDED, scope, POLICY_SOURCE_CLOUD,
policy.Set("Second", POLICY_LEVEL_RECOMMENDED, scope, source,
std::make_unique<base::Value>("maybe"), nullptr);
}
......@@ -128,14 +130,11 @@ class ComponentCloudPolicyStoreTest : public testing::Test {
return builder_.GetBlob();
}
std::unique_ptr<ComponentCloudPolicyStore> CreateStore() {
return CreateStoreWithPolicyType(dm_protocol::kChromeExtensionPolicyType);
}
std::unique_ptr<ComponentCloudPolicyStore> CreateStoreWithPolicyType(
const std::string& policy_type) {
std::unique_ptr<ComponentCloudPolicyStore> CreateStore(
const std::string& policy_type = dm_protocol::kChromeExtensionPolicyType,
const PolicySource& source = POLICY_SOURCE_CLOUD) {
return std::make_unique<ComponentCloudPolicyStore>(
&store_delegate_, cache_.get(), policy_type);
&store_delegate_, cache_.get(), policy_type, source);
}
// Returns true if the policy exposed by the |store| is empty.
......@@ -433,22 +432,21 @@ TEST_F(ComponentCloudPolicyStoreTest,
PolicyNamespace ns_signin_extension(POLICY_DOMAIN_SIGNIN_EXTENSIONS,
kTestExtension);
store_ = CreateStoreWithPolicyType(
dm_protocol::kChromeMachineLevelExtensionCloudPolicyType);
store_ =
CreateStore(dm_protocol::kChromeMachineLevelExtensionCloudPolicyType);
EXPECT_FALSE(store_->ValidatePolicy(ns_chrome, CreateResponse(),
nullptr /*policy_data*/,
nullptr /*payload*/));
EXPECT_FALSE(store_->ValidatePolicy(ns_signin_extension, CreateResponse(),
nullptr, nullptr));
store_ =
CreateStoreWithPolicyType(dm_protocol::kChromeSigninExtensionPolicyType);
store_ = CreateStore(dm_protocol::kChromeSigninExtensionPolicyType);
EXPECT_FALSE(
store_->ValidatePolicy(ns_chrome, CreateResponse(), nullptr, nullptr));
EXPECT_FALSE(
store_->ValidatePolicy(ns_extension, CreateResponse(), nullptr, nullptr));
store_ = CreateStoreWithPolicyType(dm_protocol::kChromeExtensionPolicyType);
store_ = CreateStore(dm_protocol::kChromeExtensionPolicyType);
EXPECT_FALSE(
store_->ValidatePolicy(ns_chrome, CreateResponse(), nullptr, nullptr));
EXPECT_FALSE(store_->ValidatePolicy(ns_signin_extension, CreateResponse(),
......@@ -522,8 +520,8 @@ TEST_F(ComponentCloudPolicyStoreTest, StoreAndLoad) {
}
TEST_F(ComponentCloudPolicyStoreTest, StoreAndLoadMachineLevelUserPolicy) {
store_ = CreateStoreWithPolicyType(
dm_protocol::kChromeMachineLevelExtensionCloudPolicyType);
store_ =
CreateStore(dm_protocol::kChromeMachineLevelExtensionCloudPolicyType);
store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(),
PolicyBuilder::kFakeToken,
PolicyBuilder::kFakeDeviceId, public_key_,
......@@ -536,8 +534,66 @@ TEST_F(ComponentCloudPolicyStoreTest, StoreAndLoadMachineLevelUserPolicy) {
StoreTestPolicyWithNamespace(store_.get(), kTestPolicyNS);
another_store_ = CreateStoreWithPolicyType(
another_store_ =
CreateStore(dm_protocol::kChromeMachineLevelExtensionCloudPolicyType);
another_store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(),
PolicyBuilder::kFakeToken,
PolicyBuilder::kFakeDeviceId, public_key_,
PolicyBuilder::kFakePublicKeyVersion);
another_store_->Load();
EXPECT_TRUE(another_store_->policy().Equals(expected_bundle_));
EXPECT_EQ(TestPolicyHash(), another_store_->GetCachedHash(kTestPolicyNS));
}
TEST_F(ComponentCloudPolicyStoreTest, StoreAndLoadPolicyWithCloudPriority) {
store_ = CreateStore(dm_protocol::kChromeMachineLevelExtensionCloudPolicyType,
POLICY_SOURCE_PRIORITY_CLOUD);
store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(),
PolicyBuilder::kFakeToken,
PolicyBuilder::kFakeDeviceId, public_key_,
PolicyBuilder::kFakePublicKeyVersion);
builder_.policy_data().set_policy_type(
dm_protocol::kChromeMachineLevelExtensionCloudPolicyType);
builder_.payload().set_secure_hash(TestPolicyHash());
SetupExpectBundleWithScope(POLICY_SCOPE_MACHINE,
POLICY_SOURCE_PRIORITY_CLOUD);
StoreTestPolicyWithNamespace(store_.get(), kTestPolicyNS);
another_store_ =
CreateStore(dm_protocol::kChromeMachineLevelExtensionCloudPolicyType,
POLICY_SOURCE_PRIORITY_CLOUD);
another_store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(),
PolicyBuilder::kFakeToken,
PolicyBuilder::kFakeDeviceId, public_key_,
PolicyBuilder::kFakePublicKeyVersion);
another_store_->Load();
EXPECT_TRUE(another_store_->policy().Equals(expected_bundle_));
EXPECT_EQ(TestPolicyHash(), another_store_->GetCachedHash(kTestPolicyNS));
}
TEST_F(ComponentCloudPolicyStoreTest,
StoreAndLoadPolicyWithDifferentCloudPriority) {
store_ = CreateStore(dm_protocol::kChromeMachineLevelExtensionCloudPolicyType,
POLICY_SOURCE_CLOUD);
store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(),
PolicyBuilder::kFakeToken,
PolicyBuilder::kFakeDeviceId, public_key_,
PolicyBuilder::kFakePublicKeyVersion);
builder_.policy_data().set_policy_type(
dm_protocol::kChromeMachineLevelExtensionCloudPolicyType);
builder_.payload().set_secure_hash(TestPolicyHash());
SetupExpectBundleWithScope(POLICY_SCOPE_MACHINE, POLICY_SOURCE_CLOUD);
StoreTestPolicyWithNamespace(store_.get(), kTestPolicyNS);
SetupExpectBundleWithScope(POLICY_SCOPE_MACHINE,
POLICY_SOURCE_PRIORITY_CLOUD);
another_store_ =
CreateStore(dm_protocol::kChromeMachineLevelExtensionCloudPolicyType,
POLICY_SOURCE_PRIORITY_CLOUD);
another_store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(),
PolicyBuilder::kFakeToken,
PolicyBuilder::kFakeDeviceId, public_key_,
......
......@@ -121,7 +121,8 @@ void ComponentCloudPolicyUpdaterTest::SetUp() {
task_env_.GetMainThreadTaskRunner(),
/* max_cache_size */ base::nullopt);
store_ = std::make_unique<ComponentCloudPolicyStore>(
&store_delegate_, cache_.get(), dm_protocol::kChromeExtensionPolicyType);
&store_delegate_, cache_.get(), dm_protocol::kChromeExtensionPolicyType,
POLICY_SOURCE_CLOUD);
store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(),
PolicyBuilder::kFakeToken,
PolicyBuilder::kFakeDeviceId, public_key_,
......
......@@ -50,8 +50,11 @@ void MachineLevelUserCloudPolicyManager::Connect(
CreateComponentCloudPolicyService(
dm_protocol::kChromeMachineLevelExtensionCloudPolicyType,
policy_dir_.Append(kComponentPolicyCache), client.get(),
schema_registry());
policy_dir_.Append(kComponentPolicyCache),
(local_state->GetBoolean(policy_prefs::kCloudPolicyOverridesMachinePolicy)
? POLICY_SOURCE_PRIORITY_CLOUD
: POLICY_SOURCE_CLOUD),
client.get(), schema_registry());
core()->Connect(std::move(client));
core()->StartRefreshScheduler();
core()->TrackRefreshDelayPref(local_state,
......
......@@ -20,6 +20,7 @@
#include "components/policy/core/common/policy_pref_names.h"
#include "components/policy/core/common/policy_types.h"
#include "components/policy/policy_constants.h"
#include "components/prefs/pref_service.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
namespace em = enterprise_management;
......@@ -71,9 +72,12 @@ void UserCloudPolicyManager::Connect(
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory =
client->GetURLLoaderFactory();
CreateComponentCloudPolicyService(dm_protocol::kChromeExtensionPolicyType,
component_policy_cache_path_, client.get(),
schema_registry());
CreateComponentCloudPolicyService(
dm_protocol::kChromeExtensionPolicyType, component_policy_cache_path_,
(local_state->GetBoolean(policy_prefs::kCloudPolicyOverridesMachinePolicy)
? POLICY_SOURCE_PRIORITY_CLOUD
: POLICY_SOURCE_CLOUD),
client.get(), schema_registry());
core()->Connect(std::move(client));
core()->StartRefreshScheduler();
core()->TrackRefreshDelayPref(local_state,
......
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