Restrict policy component command line switches.

The command line swtiches
--device-management-url and --realtime-reporting-url are disabled on
Window, Mac, Linux, Android on Beta and Stable channel.

They are still allowed in Dev, Canary, trunk and browser tests.

The are still allowed on Chrome OS without any additional condition.

Bug: 1119515
Change-Id: I8b78b030e06e4339e374387004d4e8e37ab863f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2382870
Commit-Queue: Owen Min <zmin@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805832}

chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc

@@ -477,6 +477,10 @@ void BrowserPolicyConnectorChromeOS::OnDeviceCloudPolicyManagerDisconnected() {
   RestartDeviceCloudPolicyInitializer();
 }
 
+bool BrowserPolicyConnectorChromeOS::IsCommandLineSwitchSupported() const {
+  return true;
+}
+
 std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
 BrowserPolicyConnectorChromeOS::CreatePolicyProviders() {
   auto providers = ChromeBrowserPolicyConnector::CreatePolicyProviders();

chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h

@@ -226,6 +226,10 @@ class BrowserPolicyConnectorChromeOS
 
   chromeos::AffiliationIDSet GetDeviceAffiliationIDs() const;
 
+  // BrowserPolicyConnector:
+  // Always returns true as command line flag can be set under dev mode only.
+  bool IsCommandLineSwitchSupported() const override;
+
  protected:
   // ChromeBrowserPolicyConnector:
   std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>

chrome/browser/chromeos/policy/system_log_uploader.cc

@@ -28,6 +28,7 @@
 #include "chrome/browser/chromeos/policy/upload_job_impl.h"
 #include "chrome/browser/device_identity/device_oauth2_token_service.h"
 #include "chrome/browser/device_identity/device_oauth2_token_service_factory.h"
+#include "chrome/browser/policy/chrome_browser_policy_connector.h"
 #include "chrome/browser/policy/chrome_policy_conversions_client.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/chrome_features.h"
@@ -267,7 +268,8 @@ base::TimeDelta GetUploadFrequency() {
 }
 
 std::string GetUploadUrl() {
-  return BrowserPolicyConnector::GetDeviceManagementUrl() +
+  return g_browser_process->browser_policy_connector()
+             ->GetDeviceManagementUrl() +
          kSystemLogUploadUrlTail;
 }
 

chrome/browser/policy/chrome_browser_policy_connector.cc

@@ -32,6 +32,7 @@
 #include "components/policy/core/common/policy_service.h"
 #include "components/policy/core/common/policy_types.h"
 #include "components/policy/policy_constants.h"
+#include "content/public/common/content_switches.h"
 #include "extensions/buildflags/buildflags.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
@@ -61,6 +62,9 @@
 #endif
 
 namespace policy {
+namespace {
+bool command_line_enabled_for_testing = false;
+}  // namespace
 
 ChromeBrowserPolicyConnector::ChromeBrowserPolicyConnector()
     : BrowserPolicyConnector(base::Bind(&BuildHandlerList)) {
@@ -81,9 +85,8 @@ void ChromeBrowserPolicyConnector::Init(
     PrefService* local_state,
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
   std::unique_ptr<DeviceManagementService::Configuration> configuration(
-      new DeviceManagementServiceConfiguration(
-          BrowserPolicyConnector::GetDeviceManagementUrl(),
-          BrowserPolicyConnector::GetRealtimeReportingUrl()));
+      new DeviceManagementServiceConfiguration(GetDeviceManagementUrl(),
+                                               GetRealtimeReportingUrl()));
   std::unique_ptr<DeviceManagementService> device_management_service(
       new DeviceManagementService(std::move(configuration)));
   device_management_service->ScheduleInitialization(
@@ -126,6 +129,20 @@ ChromeBrowserPolicyConnector::GetPlatformProvider() {
   return provider ? provider : platform_provider_;
 }
 
+bool ChromeBrowserPolicyConnector::IsCommandLineSwitchSupported() const {
+  if (command_line_enabled_for_testing)
+    return true;
+
+  version_info::Channel channel = chrome::GetChannel();
+  return channel != version_info::Channel::STABLE &&
+         channel != version_info::Channel::BETA;
+}
+
+// static
+void ChromeBrowserPolicyConnector::EnableCommandLineSupportForTesting() {
+  command_line_enabled_for_testing = true;
+}
+
 std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
 ChromeBrowserPolicyConnector::CreatePolicyProviders() {
   auto providers = BrowserPolicyConnector::CreatePolicyProviders();

chrome/browser/policy/chrome_browser_policy_connector.h

@@ -66,8 +66,15 @@ class ChromeBrowserPolicyConnector : public BrowserPolicyConnector {
   }
 #endif
 
- protected:
   // BrowserPolicyConnector:
+  // Command line switch only supports Dev and Canary channel, trunk and browser
+  // tests on Win, Mac, Linux and Android.
+  bool IsCommandLineSwitchSupported() const override;
+
+  static void EnableCommandLineSupportForTesting();
+
+ protected:
+  // BrowserPolicyConnectorBase::
   std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
   CreatePolicyProviders() override;
 

chrome/browser/policy/cloud/chrome_browser_cloud_management_browsertest.cc

@@ -462,6 +462,7 @@ class ChromeBrowserCloudManagementEnrollmentTest
     base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
     command_line->AppendSwitchASCII(switches::kDeviceManagementUrl,
                                     test_server_.GetServiceURL().spec());
+    ChromeBrowserPolicyConnector::EnableCommandLineSupportForTesting();
 
     histogram_tester_.ExpectTotalCount(kEnrollmentResultMetrics, 0);
   }
@@ -602,6 +603,7 @@ class MachineLevelUserCloudPolicyPolicyFetchTest
     base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
     command_line->AppendSwitchASCII(switches::kDeviceManagementUrl,
                                     test_server_->GetServiceURL().spec());
+    ChromeBrowserPolicyConnector::EnableCommandLineSupportForTesting();
   }
 
 #if !BUILDFLAG(GOOGLE_CHROME_BRANDING)
@@ -762,6 +764,7 @@ class MachineLevelUserCloudPolicyRobotAuthTest
     base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
     command_line->AppendSwitchASCII(switches::kDeviceManagementUrl,
                                     test_server_->GetServiceURL().spec());
+    ChromeBrowserPolicyConnector::EnableCommandLineSupportForTesting();
   }
 
 #if !BUILDFLAG(GOOGLE_CHROME_BRANDING)

chrome/browser/policy/cloud/cloud_policy_browsertest.cc

@@ -197,6 +197,7 @@ class CloudPolicyTest : public InProcessBrowserTest,
 
     base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
     command_line->AppendSwitchASCII(switches::kDeviceManagementUrl, url);
+    ChromeBrowserPolicyConnector::EnableCommandLineSupportForTesting();
   }
 
   void CreatedBrowserMainParts(

chrome/browser/policy/cloud/cloud_policy_manager_browsertest.cc

@@ -151,6 +151,7 @@ class CloudPolicyManagerTest : public InProcessBrowserTest {
     base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
     command_line->AppendSwitchASCII(switches::kDeviceManagementUrl,
                                     "http://localhost");
+    ChromeBrowserPolicyConnector::EnableCommandLineSupportForTesting();
 
     // Set retry delay to prevent timeouts.
     policy::DeviceManagementService::SetRetryDelayForTesting(0);

chrome/browser/policy/cloud/component_cloud_policy_browsertest.cc

@@ -122,6 +122,7 @@ class ComponentCloudPolicyTest : public extensions::ExtensionBrowserTest {
     std::string url = test_server_.GetServiceURL().spec();
     base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
     command_line->AppendSwitchASCII(switches::kDeviceManagementUrl, url);
+    ChromeBrowserPolicyConnector::EnableCommandLineSupportForTesting();
 
     extensions::ExtensionBrowserTest::SetUpInProcessBrowserTestFixture();
   }

chrome/browser/policy/cloud/user_policy_signin_service_browsertest.cc

@@ -9,6 +9,7 @@
 #include "base/command_line.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/policy/chrome_browser_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/ui/browser.h"
@@ -195,6 +196,7 @@ class UserPolicySigninServiceTest : public InProcessBrowserTest {
     command_line->AppendSwitchASCII(switches::kGaiaUrl, base_url.spec());
     command_line->AppendSwitchASCII(switches::kLsoUrl, base_url.spec());
     command_line->AppendSwitchASCII(switches::kGoogleApisUrl, base_url.spec());
+    policy::ChromeBrowserPolicyConnector::EnableCommandLineSupportForTesting();
     fake_gaia_.Initialize();
     // Configure Sync server.
     command_line->AppendSwitch(switches::kDisableSync);

components/policy/core/browser/browser_policy_connector.cc

@@ -135,6 +135,24 @@ bool BrowserPolicyConnector::ProviderHasPolicies(
   return false;
 }
 
+std::string BrowserPolicyConnector::GetDeviceManagementUrl() const {
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kDeviceManagementUrl) &&
+      IsCommandLineSwitchSupported())
+    return command_line->GetSwitchValueASCII(switches::kDeviceManagementUrl);
+  else
+    return kDefaultDeviceManagementServerUrl;
+}
+
+std::string BrowserPolicyConnector::GetRealtimeReportingUrl() const {
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kRealtimeReportingUrl) &&
+      IsCommandLineSwitchSupported())
+    return command_line->GetSwitchValueASCII(switches::kRealtimeReportingUrl);
+  else
+    return kDefaultRealtimeReportingServerUrl;
+}
+
 // static
 bool BrowserPolicyConnector::IsNonEnterpriseUser(const std::string& username) {
   TRACE_EVENT0("browser", "BrowserPolicyConnector::IsNonEnterpriseUser");
@@ -165,24 +183,6 @@ void BrowserPolicyConnector::SetNonEnterpriseDomainForTesting(
   non_managed_domain_for_testing = domain;
 }
 
-// static
-std::string BrowserPolicyConnector::GetDeviceManagementUrl() {
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kDeviceManagementUrl))
-    return command_line->GetSwitchValueASCII(switches::kDeviceManagementUrl);
-  else
-    return kDefaultDeviceManagementServerUrl;
-}
-
-// static
-std::string BrowserPolicyConnector::GetRealtimeReportingUrl() {
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kRealtimeReportingUrl))
-    return command_line->GetSwitchValueASCII(switches::kRealtimeReportingUrl);
-  else
-    return kDefaultRealtimeReportingServerUrl;
-}
-
 // static
 void BrowserPolicyConnector::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterIntegerPref(

components/policy/core/browser/browser_policy_connector.h

@@ -57,6 +57,12 @@ class POLICY_EXPORT BrowserPolicyConnector : public BrowserPolicyConnectorBase {
     return device_management_service_.get();
   }
 
+  // Returns the URL for the device management service endpoint.
+  std::string GetDeviceManagementUrl() const;
+
+  // Returns the URL for the realtime reporting service endpoint.
+  std::string GetRealtimeReportingUrl() const;
+
   // Check whether a user is known to be non-enterprise. Domains such as
   // gmail.com and googlemail.com are known to not be managed. Also returns
   // false if the username is empty.
@@ -67,15 +73,12 @@ class POLICY_EXPORT BrowserPolicyConnector : public BrowserPolicyConnectorBase {
   // with a nullptr.
   static void SetNonEnterpriseDomainForTesting(const char* domain);
 
-  // Returns the URL for the device management service endpoint.
-  static std::string GetDeviceManagementUrl();
-
-  // Returns the URL for the realtime reporting service endpoint.
-  static std::string GetRealtimeReportingUrl();
-
   // Registers refresh rate prefs.
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
+  // Returns true if the command line switch of policy can be used.
+  virtual bool IsCommandLineSwitchSupported() const = 0;
+
  protected:
   // Builds an uninitialized BrowserPolicyConnector.
   // Init() should be called to create and start the policy components.

ios/chrome/browser/policy/browser_policy_connector_ios.h

@@ -58,6 +58,11 @@ class BrowserPolicyConnectorIOS : public policy::BrowserPolicyConnector {
   bool HasMachineLevelPolicies() override;
   void Shutdown() override;
 
+  // BrowserPolicyConnector.
+  // Always returns true because there is no way for normal users to use command
+  // line switch anyway.
+  bool IsCommandLineSwitchSupported() const override;
+
  protected:
   // BrowserPolicyConnectorBase.
   std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>

ios/chrome/browser/policy/browser_policy_connector_ios.mm

@@ -58,8 +58,7 @@ void BrowserPolicyConnectorIOS::Init(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
   std::unique_ptr<policy::DeviceManagementService::Configuration> configuration(
       new policy::DeviceManagementServiceConfigurationIOS(
-          BrowserPolicyConnector::GetDeviceManagementUrl(),
-          BrowserPolicyConnector::GetRealtimeReportingUrl()));
+          GetDeviceManagementUrl(), GetRealtimeReportingUrl()));
   std::unique_ptr<policy::DeviceManagementService> device_management_service(
       new policy::DeviceManagementService(std::move(configuration)));
   device_management_service->ScheduleInitialization(
@@ -86,6 +85,10 @@ void BrowserPolicyConnectorIOS::Shutdown() {
   BrowserPolicyConnector::Shutdown();
 }
 
+bool BrowserPolicyConnectorIOS::IsCommandLineSwitchSupported() const {
+  return true;
+}
+
 std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
 BrowserPolicyConnectorIOS::CreatePolicyProviders() {
   auto providers = BrowserPolicyConnector::CreatePolicyProviders();