Refactor PolicyBuilder

Renames ComponentPolicyBuilder to ComponentCloudPolicyBuilder and adds
StringPolicyBuilder == ComponentActiveDirectoryPolicyBuilder, which
has a string payload instead of a protobuf payload. Active Directory
managed Chrome OS devices are going to use that for tests.

BUG=chromium:735100
TEST=Tryjobs

Change-Id: Id1f0591559d70d7e596e756d8ca5a8a588499a72
Reviewed-on: https://chromium-review.googlesource.com/1032559
Commit-Queue: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554705}

chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc

@@ -334,14 +334,14 @@ class SigninExtensionsDeviceCloudPolicyBrowserTestBase
   }
 
   static enterprise_management::PolicyFetchResponse BuildTestComponentPolicy() {
-    ComponentPolicyBuilder builder;
+    ComponentCloudPolicyBuilder builder;
     MakeTestComponentPolicyBuilder(&builder);
     return builder.policy();
   }
 
   static enterprise_management::ExternalPolicyData
   BuildTestComponentPolicyPayload() {
-    ComponentPolicyBuilder builder;
+    ComponentCloudPolicyBuilder builder;
     MakeTestComponentPolicyBuilder(&builder);
     return builder.payload();
   }
@@ -354,7 +354,8 @@ class SigninExtensionsDeviceCloudPolicyBrowserTestBase
     session_manager_client()->set_device_policy(device_policy()->GetBlob());
   }
 
-  static void MakeTestComponentPolicyBuilder(ComponentPolicyBuilder* builder) {
+  static void MakeTestComponentPolicyBuilder(
+      ComponentCloudPolicyBuilder* builder) {
     builder->policy_data().set_policy_type(
         dm_protocol::kChromeSigninExtensionPolicyType);
     builder->policy_data().set_settings_entity_id(kTestExtensionId);

components/policy/core/common/cloud/component_cloud_policy_service_unittest.cc

@@ -150,9 +150,7 @@ class ComponentCloudPolicyServiceTest : public testing::Test {
     RunUntilIdle();
   }
 
-  void RunUntilIdle() {
-    base::RunLoop().RunUntilIdle();
-  }
+  void RunUntilIdle() { base::RunLoop().RunUntilIdle(); }
 
   void Connect() {
     client_ = new MockCloudPolicyClient();
@@ -161,7 +159,7 @@ class ComponentCloudPolicyServiceTest : public testing::Test {
         client_, std::move(owned_cache_), request_context_, loop_.task_runner(),
         loop_.task_runner()));
 
-    client_->SetDMToken(ComponentPolicyBuilder::kFakeToken);
+    client_->SetDMToken(ComponentCloudPolicyBuilder::kFakeToken);
     EXPECT_EQ(1u, client_->types_to_fetch_.size());
     core_.Connect(std::unique_ptr<CloudPolicyClient>(client_));
     EXPECT_EQ(2u, client_->types_to_fetch_.size());
@@ -177,10 +175,10 @@ class ComponentCloudPolicyServiceTest : public testing::Test {
 
   void LoadStore() {
     em::PolicyData* data = new em::PolicyData();
-    data->set_username(ComponentPolicyBuilder::kFakeUsername);
-    data->set_request_token(ComponentPolicyBuilder::kFakeToken);
-    data->set_device_id(ComponentPolicyBuilder::kFakeDeviceId);
-    data->set_public_key_version(ComponentPolicyBuilder::kFakePublicKeyVersion);
+    data->set_username(PolicyBuilder::kFakeUsername);
+    data->set_request_token(PolicyBuilder::kFakeToken);
+    data->set_device_id(PolicyBuilder::kFakeDeviceId);
+    data->set_public_key_version(PolicyBuilder::kFakePublicKeyVersion);
     store_.policy_.reset(data);
     store_.policy_signature_public_key_ = public_key_;
     store_.NotifyStoreLoaded();
@@ -196,14 +194,14 @@ class ComponentCloudPolicyServiceTest : public testing::Test {
   }
 
   void PopulateCache() {
-    EXPECT_TRUE(cache_->Store(
-        "extension-policy", kTestExtension, CreateSerializedResponse()));
+    EXPECT_TRUE(cache_->Store("extension-policy", kTestExtension,
+                              CreateSerializedResponse()));
     EXPECT_TRUE(
         cache_->Store("extension-policy-data", kTestExtension, kTestPolicy));
 
     builder_.policy_data().set_settings_entity_id(kTestExtension2);
-    EXPECT_TRUE(cache_->Store(
-        "extension-policy", kTestExtension2, CreateSerializedResponse()));
+    EXPECT_TRUE(cache_->Store("extension-policy", kTestExtension2,
+                              CreateSerializedResponse()));
     EXPECT_TRUE(
         cache_->Store("extension-policy-data", kTestExtension2, kTestPolicy));
     builder_.policy_data().set_settings_entity_id(kTestExtension);
@@ -245,7 +243,7 @@ class ComponentCloudPolicyServiceTest : public testing::Test {
   CloudPolicyCore core_;
   SchemaRegistry registry_;
   std::unique_ptr<ComponentCloudPolicyService> service_;
-  ComponentPolicyBuilder builder_;
+  ComponentCloudPolicyBuilder builder_;
   PolicyMap expected_policy_;
   std::string public_key_;
 };
@@ -580,10 +578,10 @@ TEST_F(ComponentCloudPolicyServiceTest, LoadInvalidPolicyFromCache) {
   // loaded, the other should be filtered out by the schema.
   builder_.payload().set_secure_hash(
       crypto::SHA256HashString(kInvalidTestPolicy));
-  EXPECT_TRUE(cache_->Store(
-      "extension-policy", kTestExtension, CreateSerializedResponse()));
-  EXPECT_TRUE(cache_->Store(
-      "extension-policy-data", kTestExtension, kInvalidTestPolicy));
+  EXPECT_TRUE(cache_->Store("extension-policy", kTestExtension,
+                            CreateSerializedResponse()));
+  EXPECT_TRUE(cache_->Store("extension-policy-data", kTestExtension,
+                            kInvalidTestPolicy));
 
   LoadStore();
   InitializeRegistry();
@@ -661,10 +659,9 @@ TEST_F(ComponentCloudPolicyServiceTest, KeyRotation) {
   EXPECT_TRUE(service_->is_initialized());
 
   // Send back a fake policy fetch response with the new signing key.
-  const int kNewPublicKeyVersion =
-      ComponentPolicyBuilder::kFakePublicKeyVersion + 1;
+  const int kNewPublicKeyVersion = PolicyBuilder::kFakePublicKeyVersion + 1;
   std::unique_ptr<crypto::RSAPrivateKey> new_signing_key =
-      ComponentPolicyBuilder::CreateTestOtherSigningKey();
+      PolicyBuilder::CreateTestOtherSigningKey();
   builder_.SetSigningKey(*new_signing_key);
   builder_.policy_data().set_public_key_version(kNewPublicKeyVersion);
   client_->SetPolicy(dm_protocol::kChromeExtensionPolicyType, kTestExtension,

components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc

@@ -85,7 +85,7 @@ class ComponentCloudPolicyUpdaterTest : public testing::Test {
   MockComponentCloudPolicyStoreDelegate store_delegate_;
   net::TestURLFetcherFactory fetcher_factory_;
   std::unique_ptr<ComponentCloudPolicyUpdater> updater_;
-  ComponentPolicyBuilder builder_;
+  ComponentCloudPolicyBuilder builder_;
   PolicyBundle expected_bundle_;
 
  private:
@@ -119,17 +119,15 @@ void ComponentCloudPolicyUpdaterTest::SetUp() {
   task_runner_ = new base::TestMockTimeTaskRunner();
   cache_.reset(new ResourceCache(temp_dir_.GetPath(), task_runner_));
   store_.reset(new ComponentCloudPolicyStore(&store_delegate_, cache_.get()));
-  store_->SetCredentials(ComponentPolicyBuilder::GetFakeAccountIdForTesting(),
-                         ComponentPolicyBuilder::kFakeToken,
-                         ComponentPolicyBuilder::kFakeDeviceId, public_key_,
-                         ComponentPolicyBuilder::kFakePublicKeyVersion);
+  store_->SetCredentials(PolicyBuilder::GetFakeAccountIdForTesting(),
+                         PolicyBuilder::kFakeToken,
+                         PolicyBuilder::kFakeDeviceId, public_key_,
+                         PolicyBuilder::kFakePublicKeyVersion);
   fetcher_factory_.set_remove_fetcher_on_delete(true);
   fetcher_backend_.reset(new ExternalPolicyDataFetcherBackend(
-      task_runner_,
-      scoped_refptr<net::URLRequestContextGetter>()));
+      task_runner_, scoped_refptr<net::URLRequestContextGetter>()));
   updater_.reset(new ComponentCloudPolicyUpdater(
-      task_runner_,
-      fetcher_backend_->CreateFrontend(task_runner_),
+      task_runner_, fetcher_backend_->CreateFrontend(task_runner_),
       store_.get()));
   ASSERT_EQ(store_->policy().end(), store_->policy().begin());
 }
@@ -205,8 +203,8 @@ TEST_F(ComponentCloudPolicyUpdaterTest, PolicyFetchResponseInvalid) {
   updater_->UpdateExternalPolicy(kTestPolicyNS, CreateResponse());
 
   // Submit two valid policy fetch responses.
-  builder_.policy_data().set_username(ComponentPolicyBuilder::kFakeUsername);
-  builder_.policy_data().set_gaia_id(ComponentPolicyBuilder::kFakeGaiaId);
+  builder_.policy_data().set_username(PolicyBuilder::kFakeUsername);
+  builder_.policy_data().set_gaia_id(PolicyBuilder::kFakeGaiaId);
   builder_.policy_data().set_settings_entity_id(kTestExtension2);
   builder_.payload().set_download_url(kTestDownload2);
   updater_->UpdateExternalPolicy(
@@ -255,7 +253,7 @@ TEST_F(ComponentCloudPolicyUpdaterTest, PolicyFetchResponseBadSignature) {
 
 TEST_F(ComponentCloudPolicyUpdaterTest, PolicyFetchResponseWrongPublicKey) {
   // Submit a policy fetch response signed with a wrong signing key.
-  builder_.SetSigningKey(*ComponentPolicyBuilder::CreateTestOtherSigningKey());
+  builder_.SetSigningKey(*PolicyBuilder::CreateTestOtherSigningKey());
   updater_->UpdateExternalPolicy(kTestPolicyNS, CreateResponse());
 
   task_runner_->RunUntilIdle();
@@ -268,7 +266,7 @@ TEST_F(ComponentCloudPolicyUpdaterTest,
        PolicyFetchResponseWrongPublicKeyVersion) {
   // Submit a policy fetch response containing different public key version.
   builder_.policy_data().set_public_key_version(
-      ComponentPolicyBuilder::kFakePublicKeyVersion + 1);
+      PolicyBuilder::kFakePublicKeyVersion + 1);
   updater_->UpdateExternalPolicy(kTestPolicyNS, CreateResponse());
 
   task_runner_->RunUntilIdle();
@@ -280,9 +278,9 @@ TEST_F(ComponentCloudPolicyUpdaterTest,
 TEST_F(ComponentCloudPolicyUpdaterTest, PolicyFetchResponseDifferentPublicKey) {
   // Submit a policy fetch response signed with a different key and containing a
   // new public key version.
-  builder_.SetSigningKey(*ComponentPolicyBuilder::CreateTestOtherSigningKey());
+  builder_.SetSigningKey(*PolicyBuilder::CreateTestOtherSigningKey());
   builder_.policy_data().set_public_key_version(
-      ComponentPolicyBuilder::kFakePublicKeyVersion + 1);
+      PolicyBuilder::kFakePublicKeyVersion + 1);
   updater_->UpdateExternalPolicy(kTestPolicyNS, CreateResponse());
 
   task_runner_->RunUntilIdle();

components/policy/core/common/cloud/policy_builder.cc

@@ -251,8 +251,7 @@ void PolicyBuilder::Build() {
     // The new public key must be signed by the old key.
     std::unique_ptr<crypto::RSAPrivateKey> old_signing_key = GetSigningKey();
     if (old_signing_key) {
-      SignData(policy_.new_public_key(),
-               old_signing_key.get(),
+      SignData(policy_.new_public_key(), old_signing_key.get(),
                policy_.mutable_new_public_key_signature());
     }
   } else {
@@ -370,7 +369,7 @@ AccountId PolicyBuilder::GetFakeAccountIdForTesting() {
   return AccountId::FromUserEmailGaiaId(kFakeUsername, kFakeGaiaId);
 }
 
-template<>
+template <>
 TypedPolicyBuilder<em::CloudPolicySettings>::TypedPolicyBuilder()
     : payload_(new em::CloudPolicySettings()) {
   policy_data().set_policy_type(dm_protocol::kChromeUserPolicyType);
@@ -380,7 +379,7 @@ TypedPolicyBuilder<em::CloudPolicySettings>::TypedPolicyBuilder()
 template class TypedPolicyBuilder<em::CloudPolicySettings>;
 
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
-template<>
+template <>
 TypedPolicyBuilder<em::ExternalPolicyData>::TypedPolicyBuilder() {
   CreatePayload();
   policy_data().set_policy_type(dm_protocol::kChromeExtensionPolicyType);
@@ -389,4 +388,13 @@ TypedPolicyBuilder<em::ExternalPolicyData>::TypedPolicyBuilder() {
 template class TypedPolicyBuilder<em::ExternalPolicyData>;
 #endif
 
+#if defined(OS_CHROMEOS)
+StringPolicyBuilder::StringPolicyBuilder() = default;
+
+void StringPolicyBuilder::Build() {
+  policy_data().set_policy_value(payload_);
+  PolicyBuilder::Build();
+}
+#endif
+
 }  // namespace policy

components/policy/core/common/cloud/policy_builder.h

@@ -152,7 +152,7 @@ class PolicyBuilder {
 
 // Type-parameterized PolicyBuilder extension that allows for building policy
 // blobs carrying protobuf payloads.
-template<typename PayloadProto>
+template <typename PayloadProto>
 class TypedPolicyBuilder : public PolicyBuilder {
  public:
   TypedPolicyBuilder();
@@ -178,12 +178,34 @@ class TypedPolicyBuilder : public PolicyBuilder {
   DISALLOW_COPY_AND_ASSIGN(TypedPolicyBuilder);
 };
 
+// PolicyBuilder extension that allows for building policy blobs carrying string
+// payloads.
+class StringPolicyBuilder : public PolicyBuilder {
+ public:
+  StringPolicyBuilder();
+  void set_payload(std::string payload) { payload_ = std::move(payload); }
+  const std::string& payload() const { return payload_; }
+  void clear_payload() { payload_.clear(); }
+
+  // PolicyBuilder:
+  void Build() override;
+
+ private:
+  std::string payload_;
+
+  DISALLOW_COPY_AND_ASSIGN(StringPolicyBuilder);
+};
+
 typedef TypedPolicyBuilder<enterprise_management::CloudPolicySettings>
     UserPolicyBuilder;
 
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
-typedef TypedPolicyBuilder<enterprise_management::ExternalPolicyData>
-    ComponentPolicyBuilder;
+using ComponentCloudPolicyBuilder =
+    TypedPolicyBuilder<enterprise_management::ExternalPolicyData>;
+#endif
+
+#if defined(OS_CHROMEOS)
+using ComponentActiveDirectoryPolicyBuilder = StringPolicyBuilder;
 #endif
 
 }  // namespace policy