Add CommandLinePolicyProvider

Create the provider and link it to the PolicyConnector.

Also make few changes in the Loader class:
1) It no longer inherits from ASyncPolicyLoader as the loader process
is sync.
2) Add few debug information in case the JSON input is not valid.

The provider is not disabled globally. There will be one last CL to
enable the provider on Android.

Bug: 1113792
Change-Id: Id32ec74ebfa259777f718bf06b7a4cb307498f2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2355134Reviewed-by: Yann Dago <ydago@chromium.org>
Commit-Queue: Owen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798850}

chrome/browser/policy/chrome_browser_policy_connector.cc

@@ -24,6 +24,7 @@
 #include "components/policy/core/common/cloud/cloud_policy_client_registration_helper.h"
 #include "components/policy/core/common/cloud/device_management_service.h"
 #include "components/policy/core/common/cloud/user_cloud_policy_manager.h"
+#include "components/policy/core/common/command_line_policy_provider.h"
 #include "components/policy/core/common/configuration_policy_provider.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_namespace.h"
@@ -60,6 +61,13 @@
 
 namespace policy {
 
+namespace {
+bool IsCommandLinePolicySupported() {
+  // TODO(crbug/1113792): Enable it on Android.
+  return false;
+}
+}  // namespace
+
 ChromeBrowserPolicyConnector::ChromeBrowserPolicyConnector()
     : BrowserPolicyConnector(base::Bind(&BuildHandlerList)) {
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
@@ -102,6 +110,8 @@ bool ChromeBrowserPolicyConnector::HasMachineLevelPolicies() {
   if (ProviderHasPolicies(machine_level_user_cloud_policy_manager_))
     return true;
 #endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
+  if (ProviderHasPolicies(command_line_provider_))
+    return true;
   return false;
 }
 
@@ -145,6 +155,14 @@ ChromeBrowserPolicyConnector::CreatePolicyProviders() {
   }
 #endif
 
+  if (IsCommandLinePolicySupported()) {
+    std::unique_ptr<CommandLinePolicyProvider> command_line_provider =
+        std::make_unique<CommandLinePolicyProvider>(
+            *base::CommandLine::ForCurrentProcess());
+    command_line_provider_ = command_line_provider.get();
+    providers.push_back(std::move(command_line_provider));
+  }
+
   return providers;
 }
 

chrome/browser/policy/chrome_browser_policy_connector.h

@@ -85,6 +85,8 @@ class ChromeBrowserPolicyConnector : public BrowserPolicyConnector {
       nullptr;
 #endif
 
+  ConfigurationPolicyProvider* command_line_provider_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserPolicyConnector);
 };
 

components/policy/core/common/BUILD.gn

@@ -92,6 +92,8 @@ source_set("internal") {
     "cloud/user_cloud_policy_store_base.h",
     "cloud/user_info_fetcher.cc",
     "cloud/user_info_fetcher.h",
+    "command_line_policy_provider.cc",
+    "command_line_policy_provider.h",
     "config_dir_policy_loader.cc",
     "config_dir_policy_loader.h",
     "configuration_policy_provider.cc",
@@ -399,6 +401,7 @@ source_set("unit_tests") {
     "cloud/cloud_policy_validator_unittest.cc",
     "cloud/device_management_service_unittest.cc",
     "cloud/user_info_fetcher_unittest.cc",
+    "command_line_policy_provider_unittest.cc",
     "generate_policy_source_unittest.cc",
     "legacy_chrome_policy_migrator_unittest.cc",
     "management/management_service_unittest.cc",

components/policy/core/common/command_line_policy_provider.cc

+// 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 "components/policy/core/common/command_line_policy_provider.h"
+
+#include <memory>
+#include <utility>
+
+#include "components/policy/core/common/policy_bundle.h"
+
+namespace policy {
+
+CommandLinePolicyProvider::CommandLinePolicyProvider(
+    const base::CommandLine& command_line)
+    : loader_(command_line) {
+  RefreshPolicies();
+}
+CommandLinePolicyProvider::~CommandLinePolicyProvider() = default;
+
+void CommandLinePolicyProvider::RefreshPolicies() {
+  std::unique_ptr<PolicyBundle> bundle = loader_.Load();
+  UpdatePolicy(std::move(bundle));
+}
+
+}  // namespace policy

components/policy/core/common/command_line_policy_provider.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.
+
+#ifndef COMPONENTS_POLICY_CORE_COMMON_COMMAND_LINE_POLICY_PROVIDER_H_
+#define COMPONENTS_POLICY_CORE_COMMON_COMMAND_LINE_POLICY_PROVIDER_H_
+
+#include "components/policy/core/common/configuration_policy_provider.h"
+#include "components/policy/core/common/policy_loader_command_line.h"
+#include "components/policy/policy_export.h"
+
+namespace policy {
+
+// The policy provider for the Command Line Policy which is used for development
+// and testing purposes.
+class POLICY_EXPORT CommandLinePolicyProvider
+    : public ConfigurationPolicyProvider {
+ public:
+  explicit CommandLinePolicyProvider(const base::CommandLine& command_line);
+  CommandLinePolicyProvider(const CommandLinePolicyProvider&) = delete;
+  CommandLinePolicyProvider& operator=(const CommandLinePolicyProvider&) =
+      delete;
+
+  ~CommandLinePolicyProvider() override;
+
+  // ConfigurationPolicyProvider implementation.
+  void RefreshPolicies() override;
+
+ private:
+  PolicyLoaderCommandLine loader_;
+};
+
+}  // namespace policy
+
+#endif  // COMPONENTS_POLICY_CORE_COMMON_COMMAND_LINE_POLICY_PROVIDER_H_

components/policy/core/common/command_line_policy_provider_unittest.cc

+// 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 "components/policy/core/common/command_line_policy_provider.h"
+
+#include <memory>
+
+#include "base/values.h"
+#include "components/policy/core/common/policy_bundle.h"
+#include "components/policy/core/common/policy_switches.h"
+#include "components/policy/core/common/policy_types.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace policy {
+
+namespace {
+
+void VerifyPolicyProvider(ConfigurationPolicyProvider* provider) {
+  const base::Value* policy_value =
+      provider->policies()
+          .Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
+          .GetValue("policy");
+  ASSERT_TRUE(policy_value);
+  ASSERT_TRUE(policy_value->is_int());
+  EXPECT_EQ(10, policy_value->GetInt());
+}
+
+}  // namespace
+
+class CommandLinePolicyProviderTest : public ::testing::Test {
+ public:
+  CommandLinePolicyProviderTest() {
+    command_line_.AppendSwitchASCII(switches::kChromePolicy,
+                                    R"({"policy":10})");
+  }
+
+  std::unique_ptr<CommandLinePolicyProvider> CreatePolicyProvider() {
+    return std::make_unique<CommandLinePolicyProvider>(command_line_);
+  }
+
+ private:
+  base::CommandLine command_line_{base::CommandLine::NO_PROGRAM};
+};
+
+TEST_F(CommandLinePolicyProviderTest, LoadAndRefresh) {
+  std::unique_ptr<CommandLinePolicyProvider> policy_provider =
+      CreatePolicyProvider();
+  VerifyPolicyProvider(policy_provider.get());
+
+  policy_provider->RefreshPolicies();
+  VerifyPolicyProvider(policy_provider.get());
+}
+}  // namespace policy

components/policy/core/common/policy_loader_command_line.cc

@@ -5,6 +5,7 @@
 #include "components/policy/core/common/policy_loader_command_line.h"
 
 #include "base/json/json_reader.h"
+#include "base/logging.h"
 #include "base/sequenced_task_runner.h"
 #include "base/values.h"
 #include "components/policy/core/common/policy_bundle.h"
@@ -14,29 +15,33 @@
 namespace policy {
 
 PolicyLoaderCommandLine::PolicyLoaderCommandLine(
-    scoped_refptr<base::SequencedTaskRunner> task_runner,
     const base::CommandLine& command_line)
-    : AsyncPolicyLoader(task_runner), command_line_(command_line) {}
+    : command_line_(command_line) {}
 PolicyLoaderCommandLine::~PolicyLoaderCommandLine() = default;
 
-void PolicyLoaderCommandLine::InitOnBackgroundThread() {}
-
 std::unique_ptr<PolicyBundle> PolicyLoaderCommandLine::Load() {
   std::unique_ptr<PolicyBundle> bundle = std::make_unique<PolicyBundle>();
   if (!command_line_.HasSwitch(switches::kChromePolicy))
     return bundle;
 
-  base::Optional<base::Value> policies = base::JSONReader::Read(
-      command_line_.GetSwitchValueASCII(switches::kChromePolicy));
+  base::JSONReader::ValueWithError policies =
+      base::JSONReader::ReadAndReturnValueWithError(
+          command_line_.GetSwitchValueASCII(switches::kChromePolicy),
+          base::JSONParserOptions::JSON_ALLOW_TRAILING_COMMAS);
 
-  if (!policies || !policies->is_dict())
+  if (!policies.value) {
+    VLOG(1) << "Command line policy error: " << policies.error_message;
+    return bundle;
+  }
+  if (!policies.value->is_dict()) {
+    VLOG(1) << "Command line policy is not a dictionary";
     return bundle;
+  }
 
   bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()))
-      .LoadFrom(&base::Value::AsDictionaryValue(*policies),
+      .LoadFrom(&base::Value::AsDictionaryValue(*policies.value),
                 POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
                 POLICY_SOURCE_COMMAND_LINE);
-
   return bundle;
 }
 

components/policy/core/common/policy_loader_command_line.h

@@ -5,35 +5,31 @@
 #ifndef COMPONENTS_POLICY_CORE_COMMON_POLICY_LOADER_COMMAND_LINE_H_
 #define COMPONENTS_POLICY_CORE_COMMON_POLICY_LOADER_COMMAND_LINE_H_
 
+#include <memory>
+
 #include "base/command_line.h"
 #include "base/memory/ref_counted.h"
-#include "components/policy/core/common/async_policy_loader.h"
 #include "components/policy/policy_export.h"
 
-namespace base {
-class SequencedTaskRunner;
-}
-
 namespace policy {
 
+class PolicyBundle;
+
 // Loads policy value from command line switch for development and testing
 // purposes. It can be only used with strict limitation. For example, on
 // Android, the device must be rooted.
-class POLICY_EXPORT PolicyLoaderCommandLine : public AsyncPolicyLoader {
+class POLICY_EXPORT PolicyLoaderCommandLine {
  public:
-  PolicyLoaderCommandLine(scoped_refptr<base::SequencedTaskRunner> task_runner,
-                          const base::CommandLine& command_line);
+  explicit PolicyLoaderCommandLine(const base::CommandLine& command_line);
   PolicyLoaderCommandLine(const PolicyLoaderCommandLine&) = delete;
   PolicyLoaderCommandLine& operator=(const PolicyLoaderCommandLine&) = delete;
 
-  ~PolicyLoaderCommandLine() override;
+  ~PolicyLoaderCommandLine();
 
-  // AsumcPolicyLoader implementation.
-  void InitOnBackgroundThread() override;
-  std::unique_ptr<PolicyBundle> Load() override;
+  std::unique_ptr<PolicyBundle> Load();
 
  private:
-  const base::CommandLine command_line_;
+  const base::CommandLine& command_line_;
 };
 
 }  // namespace policy

components/policy/core/common/policy_loader_command_line_unittest.cc

@@ -40,8 +40,7 @@ class PolicyLoaderCommandLineTest : public ::testing::Test {
   }
 
   std::unique_ptr<PolicyLoaderCommandLine> CreatePolicyLoader() {
-    return std::make_unique<PolicyLoaderCommandLine>(
-        task_environment_.GetMainThreadTaskRunner(), command_line_);
+    return std::make_unique<PolicyLoaderCommandLine>(command_line_);
   }
 
  private: