Change policy::DeviceManagementServiceConfiguration to use

OnSecurityEventEnterpriseConnector policy.

Before connectors the URL for reporting was hardcode to one value.
Connectors adds the ability for the URL to change dynamically based on
the connector chosen by the admin, as identified by the connector
policy.

Bug: 1069031
Change-Id: Ic59379283d7651be45d65f6f9785c43c0d43e1ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2251742
Commit-Queue: Roger Tawa <rogerta@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Owen Min <zmin@chromium.org>
Reviewed-by: Dominique Fauteux-Chapleau <domfc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781538}

chrome/browser/enterprise/connectors/common.cc

@@ -15,6 +15,7 @@ AnalysisSettings& AnalysisSettings::operator=(AnalysisSettings&&) = default;
 AnalysisSettings::~AnalysisSettings() = default;
 
 ReportingSettings::ReportingSettings() = default;
+ReportingSettings::ReportingSettings(GURL url) : reporting_url(url) {}
 ReportingSettings::ReportingSettings(ReportingSettings&&) = default;
 ReportingSettings& ReportingSettings::operator=(ReportingSettings&&) = default;
 ReportingSettings::~ReportingSettings() = default;

chrome/browser/enterprise/connectors/common.h

@@ -54,6 +54,7 @@ struct AnalysisSettings {
 
 struct ReportingSettings {
   ReportingSettings();
+  explicit ReportingSettings(GURL url);
   ReportingSettings(ReportingSettings&&);
   ReportingSettings& operator=(ReportingSettings&&);
   ~ReportingSettings();

chrome/browser/enterprise/connectors/connectors_manager.cc

@@ -15,6 +15,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "components/url_matcher/url_matcher.h"
+#include "url/gurl.h"
 
 namespace enterprise_connectors {
 
@@ -400,7 +401,8 @@ ConnectorsManager::GetReportingSettingsFromLegacyPolicies(
     return base::nullopt;
   }
 
-  return ReportingSettings();
+  return ReportingSettings(
+      GURL("https://chromereporting-pa.googleapis.com/v1/events"));
 }
 
 void ConnectorsManager::StartObservingPrefs() {

chrome/browser/enterprise/connectors/connectors_manager_unittest.cc

@@ -132,7 +132,13 @@ class ConnectorsManagerTest : public testing::Test {
     ASSERT_EQ(settings.tags, expected_tags_);
   }
 
-  void ValidateSettings(const ReportingSettings& settings) {}
+  void ValidateSettings(const ReportingSettings& settings) {
+    // For now, the URL is the same for both legacy and new policies, so
+    // checking the specific URL here.  When service providers become
+    // configurable this will change.
+    ASSERT_EQ(GURL("https://chromereporting-pa.googleapis.com/v1/events"),
+              settings.reporting_url);
+  }
 
   class ScopedConnectorPref {
    public:

chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router.cc

@@ -783,6 +783,11 @@ void SafeBrowsingPrivateEventRouter::InitRealtimeReportingClient() {
       policy::CloudPolicyClient::DeviceDMTokenCallback());
   client = private_client_.get();
 
+  // TODO(crbug.com/1069049): when we decide to add the extra URL parameters to
+  // the uploaded reports, do the following:
+  //     client->add_connector_url_params(base::FeatureList::IsEnabled(
+  //        enterprise_connectors::kEnterpriseConnectorsEnabled));
+
   if (!client->is_registered()) {
     client->SetupRegistration(
         dm_token.value(), client_id,

chrome/browser/policy/device_management_service_configuration.cc

@@ -17,6 +17,12 @@
 #include "chromeos/system/statistics_provider.h"
 #endif
 
+#if defined(OS_WIN) || defined(OS_MACOSX) || \
+    (defined(OS_LINUX) && !defined(OS_ANDROID))
+#include "chrome/browser/enterprise/connectors/common.h"
+#include "chrome/browser/enterprise/connectors/connectors_manager.h"
+#endif
+
 namespace policy {
 
 DeviceManagementServiceConfiguration::DeviceManagementServiceConfiguration(
@@ -77,4 +83,18 @@ std::string DeviceManagementServiceConfiguration::GetReportingServerUrl() {
   return reporting_server_url_;
 }
 
+std::string
+DeviceManagementServiceConfiguration::GetReportingConnectorServerUrl() {
+#if defined(OS_WIN) || defined(OS_MACOSX) || \
+    (defined(OS_LINUX) && !defined(OS_ANDROID))
+  auto settings =
+      enterprise_connectors::ConnectorsManager::GetInstance()
+          ->GetReportingSettings(
+              enterprise_connectors::ReportingConnector::SECURITY_EVENT);
+  return settings ? settings->reporting_url.spec() : std::string();
+#else
+  return std::string();
+#endif
+}
+
 }  // namespace policy

chrome/browser/policy/device_management_service_configuration.h

@@ -18,15 +18,15 @@ namespace policy {
 class DeviceManagementServiceConfiguration
     : public DeviceManagementService::Configuration {
  public:
-  explicit DeviceManagementServiceConfiguration(
-      const std::string& server_url,
-      const std::string& reporting_server_url);
+  DeviceManagementServiceConfiguration(const std::string& server_url,
+                                       const std::string& reporting_server_url);
   ~DeviceManagementServiceConfiguration() override;
 
   std::string GetDMServerUrl() override;
   std::string GetAgentParameter() override;
   std::string GetPlatformParameter() override;
   std::string GetReportingServerUrl() override;
+  std::string GetReportingConnectorServerUrl() override;
 
  private:
   const std::string server_url_;

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

@@ -595,7 +595,10 @@ void CloudPolicyClient::UploadRealtimeReport(base::Value report,
                                              StatusCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   CHECK(is_registered());
-  CreateNewRealtimeReportingJob(std::move(report), std::move(callback));
+  CreateNewRealtimeReportingJob(
+      std::move(report),
+      service()->configuration()->GetReportingConnectorServerUrl(),
+      add_connector_url_params_, std::move(callback));
 }
 
 void CloudPolicyClient::UploadAppInstallReport(base::Value report,
@@ -603,8 +606,9 @@ void CloudPolicyClient::UploadAppInstallReport(base::Value report,
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   CHECK(is_registered());
   CancelAppInstallReportUpload();
-  app_install_report_request_job_ =
-      CreateNewRealtimeReportingJob(std::move(report), std::move(callback));
+  app_install_report_request_job_ = CreateNewRealtimeReportingJob(
+      std::move(report), service()->configuration()->GetReportingServerUrl(),
+      /* add_connector_url_params=*/false, std::move(callback));
   DCHECK(app_install_report_request_job_);
 }
 
@@ -622,8 +626,9 @@ void CloudPolicyClient::UploadExtensionInstallReport(base::Value report,
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   CHECK(is_registered());
   CancelExtensionInstallReportUpload();
-  extension_install_report_request_job_ =
-      CreateNewRealtimeReportingJob(std::move(report), std::move(callback));
+  extension_install_report_request_job_ = CreateNewRealtimeReportingJob(
+      std::move(report), service()->configuration()->GetReportingServerUrl(),
+      /* add_connector_url_params=*/false, std::move(callback));
   DCHECK(extension_install_report_request_job_);
 }
 
@@ -667,10 +672,13 @@ void CloudPolicyClient::FetchRemoteCommands(
 
 DeviceManagementService::Job* CloudPolicyClient::CreateNewRealtimeReportingJob(
     base::Value report,
+    const std::string& server_url,
+    bool add_connector_url_params,
     StatusCallback callback) {
   std::unique_ptr<RealtimeReportingJobConfiguration> config =
       std::make_unique<RealtimeReportingJobConfiguration>(
-          this, DMAuth::FromDMToken(dm_token_),
+          this, DMAuth::FromDMToken(dm_token_), server_url,
+          add_connector_url_params,
           base::BindOnce(&CloudPolicyClient::OnRealtimeReportUploadCompleted,
                          weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 

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

@@ -302,9 +302,10 @@ class POLICY_EXPORT CloudPolicyClient {
           chrome_os_user_report,
       StatusCallback callback);
 
-  // Uploads |report| using the real-time reporting API.  As above, the client
-  // must be in a registered state.  The |callback| will be called when the
-  // operation completes.
+  // Uploads a report containing enterprise connectors real-time security
+  // events. As above, the client must be in a registered state.  The |callback|
+  // will be called when the operation completes.
+  // TODO(crbug.com/1098437): Pick a more specific name.
   virtual void UploadRealtimeReport(base::Value report,
                                     StatusCallback callback);
 
@@ -551,6 +552,10 @@ class POLICY_EXPORT CloudPolicyClient {
 
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory();
 
+  void add_connector_url_params(bool value) {
+    add_connector_url_params_ = value;
+  }
+
   // Returns the number of active requests.
   int GetActiveRequestCountForTest() const;
 
@@ -763,9 +768,15 @@ class POLICY_EXPORT CloudPolicyClient {
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 
  private:
-  // Creates new realtime reporting job and appends it to |request_jobs_|.
+  // Creates a new real-time reporting job and appends it to |request_jobs_|.
+  // The job will send its report to the |server_url| endpoint.  If
+  // |add_connector_url_params| is true then URL paramaters specific to
+  // enterprise connectors are added to the request uploading the report.
+  // |callback| is invoked once the report is uploaded.
   DeviceManagementService::Job* CreateNewRealtimeReportingJob(
       base::Value report,
+      const std::string& server_url,
+      bool add_connector_url_params,
       StatusCallback callback);
 
   void SetClientId(const std::string& client_id);
@@ -796,6 +807,11 @@ class POLICY_EXPORT CloudPolicyClient {
   // error |DM_STATUS_SERVICE_DEVICE_NOT_FOUND|.
   std::string reregistration_dm_token_;
 
+  // Whether extra enterprise connectors URL parameters should be included
+  // in real-time reports.  Only reports uploaded using UploadRealtimeReport()
+  // are affected.
+  bool add_connector_url_params_ = false;
+
   // Used to create tasks which run delayed on the UI thread.
   base::WeakPtrFactory<CloudPolicyClient> weak_ptr_factory_{this};
 

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

@@ -1521,6 +1521,7 @@ TEST_F(CloudPolicyClientTest, UploadRealtimeReport) {
 TEST_F(CloudPolicyClientTest, RealtimeReportMerge) {
   auto config = std::make_unique<RealtimeReportingJobConfiguration>(
       client_.get(), DMAuth::FromDMToken(kDMToken),
+      service_.configuration()->GetReportingServerUrl(), false,
       RealtimeReportingJobConfiguration::Callback());
 
   // Add one report to the config.

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

@@ -110,6 +110,10 @@ class POLICY_EXPORT DeviceManagementService {
 
     // Server at which to contact the real time reporting service.
     virtual std::string GetReportingServerUrl() = 0;
+
+    // Server at which to contact the real time reporting service for
+    // enterprise connectors.
+    virtual std::string GetReportingConnectorServerUrl() = 0;
   };
 
   // A DeviceManagementService job manages network requests to the device

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

@@ -101,6 +101,11 @@ std::string MockDeviceManagementServiceConfiguration::GetReportingServerUrl() {
   return server_url_;
 }
 
+std::string
+MockDeviceManagementServiceConfiguration::GetReportingConnectorServerUrl() {
+  return server_url_;
+}
+
 MockDeviceManagementService::MockDeviceManagementService()
     : DeviceManagementService(std::unique_ptr<Configuration>(
           new MockDeviceManagementServiceConfiguration)) {}

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

@@ -32,6 +32,7 @@ class MockDeviceManagementServiceConfiguration
   std::string GetAgentParameter() override;
   std::string GetPlatformParameter() override;
   std::string GetReportingServerUrl() override;
+  std::string GetReportingConnectorServerUrl() override;
 
  private:
   const std::string server_url_;

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

@@ -58,19 +58,26 @@ base::Value RealtimeReportingJobConfiguration::BuildReport(
 RealtimeReportingJobConfiguration::RealtimeReportingJobConfiguration(
     CloudPolicyClient* client,
     std::unique_ptr<DMAuth> auth_data,
+    const std::string& server_url,
+    bool add_connector_url_params,
     Callback callback)
     : JobConfigurationBase(TYPE_UPLOAD_REAL_TIME_REPORT,
                            std::move(auth_data),
                            base::nullopt,
                            client->GetURLLoaderFactory()),
-      server_url_(client->service()->configuration()->GetReportingServerUrl()),
+      server_url_(server_url),
       payload_(base::Value::Type::DICTIONARY),
       callback_(std::move(callback)) {
   DCHECK(GetAuth().has_dm_token());
 
   AddParameter("key", google_apis::GetAPIKey());
-  AddParameter(enterprise::kUrlParamConnector, "OnSecurityEvent");
-  AddParameter(enterprise::kUrlParamDeviceToken, client->dm_token());
+
+  // If specified add extra enterprise connector URL params.
+  if (add_connector_url_params) {
+    AddParameter(enterprise::kUrlParamConnector, "OnSecurityEvent");
+    AddParameter(enterprise::kUrlParamDeviceToken, client->dm_token());
+  }
+
   InitializePayload(client);
 }
 

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

@@ -53,8 +53,14 @@ class POLICY_EXPORT RealtimeReportingJobConfiguration
   // google3/google/internal/chrome/reporting/v1/chromereporting.proto.
   static base::Value BuildReport(base::Value events, base::Value context);
 
+  // Configures a request to send real-time reports to the |server_url|
+  // endpoint.  If |add_connector_url_params| is true then URL parameters
+  // specific to enterprise connectors are added to the request uploading
+  // the report.  |callback| is invoked once the report is uploaded.
   RealtimeReportingJobConfiguration(CloudPolicyClient* client,
                                     std::unique_ptr<DMAuth> auth_data,
+                                    const std::string& server_url,
+                                    bool add_connector_url_params,
                                     Callback callback);
 
   ~RealtimeReportingJobConfiguration() override;

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

@@ -71,8 +71,11 @@ class RealtimeReportingJobConfigurationTest : public testing::Test {
 #endif
         configuration_(&client_,
                        DMAuth::FromDMToken(kDummyToken),
+                       service_.configuration()->GetReportingServerUrl(),
+                       false,
                        base::BindOnce(&MockCallbackObserver::OnURLLoadComplete,
-                                      base::Unretained(&callback_observer_))) {}
+                                      base::Unretained(&callback_observer_))) {
+  }
 
   void SetUp() override {
     base::Value context(base::Value::Type::DICTIONARY);