Reland "Add profile DM token reading code to ConnectorsService"

This relands crrev.com/c/2590429 with the exact same changes, except
with some changes to avoid the CFI error that caused a revert
(crrev.com/c/2597715). Those differences with the original CL can be
seen by looking at the patchset 1-3 diff.

This is a reland of 4ee6e77f

Original change's description:
> Add profile DM token reading code to ConnectorsService
>
> This adds the main code to read the DM token from the profile instead of
> CBCM. The PerProfileConnectorsEnabled feature guards these changes from
> affecting working Connector policies. This change is insufficient to
> test per-profile Connectors since the authentication and reporting code
> still assume the DM token is from the browser.
>
> This adds a tweak to Connector policy test setters to ensure existing
> tests don't break and still use the browser DM token.
>
> Bug: 1148789, 1155585
> Change-Id: I64bb1714667b0b61fdfdbadffbc153328e22f741
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2590429
> Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
> Reviewed-by: Daniel Rubery <drubery@chromium.org>
> Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#838139}

Bug: 1148789
Bug: 1155585
Change-Id: Ic69ae3760171932f1f7dde8674a90b860ce22b4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2603586Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840221}

chrome/browser/BUILD.gn

@@ -496,6 +496,8 @@ static_library("browser") {
     "enterprise/browser_management/browser_management_service.h",
     "enterprise/browser_management/browser_management_status_provider.cc",
     "enterprise/browser_management/browser_management_status_provider.h",
+    "enterprise/util/affiliation.cc",
+    "enterprise/util/affiliation.h",
     "enterprise/util/managed_browser_utils.cc",
     "enterprise/util/managed_browser_utils.h",
     "expired_flags_list.h",

chrome/browser/enterprise/connectors/common.cc

@@ -15,8 +15,10 @@ AnalysisSettings& AnalysisSettings::operator=(AnalysisSettings&&) = default;
 AnalysisSettings::~AnalysisSettings() = default;
 
 ReportingSettings::ReportingSettings() = default;
-ReportingSettings::ReportingSettings(GURL url, const std::string& dm_token)
-    : reporting_url(url), dm_token(dm_token) {}
+ReportingSettings::ReportingSettings(GURL url,
+                                     const std::string& dm_token,
+                                     bool per_profile)
+    : reporting_url(url), dm_token(dm_token), per_profile(per_profile) {}
 ReportingSettings::ReportingSettings(ReportingSettings&&) = default;
 ReportingSettings& ReportingSettings::operator=(ReportingSettings&&) = default;
 ReportingSettings::~ReportingSettings() = default;

chrome/browser/enterprise/connectors/common.h

@@ -66,13 +66,19 @@ struct AnalysisSettings {
 
 struct ReportingSettings {
   ReportingSettings();
-  explicit ReportingSettings(GURL url, const std::string& dm_token);
+  explicit ReportingSettings(GURL url,
+                             const std::string& dm_token,
+                             bool per_profile);
   ReportingSettings(ReportingSettings&&);
   ReportingSettings& operator=(ReportingSettings&&);
   ~ReportingSettings();
 
   GURL reporting_url;
   std::string dm_token;
+
+  // Indicates if the report should be made for the profile, or the browser if
+  // false.
+  bool per_profile = false;
 };
 
 // Returns the pref path corresponding to a connector.

chrome/browser/enterprise/connectors/connectors_service.cc

@@ -7,13 +7,20 @@
 
 #include "base/memory/singleton.h"
 #include "base/no_destructor.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/enterprise/connectors/common.h"
 #include "chrome/browser/enterprise/connectors/connectors_manager.h"
 #include "chrome/browser/enterprise/connectors/service_provider_config.h"
+#include "chrome/browser/enterprise/util/affiliation.h"
+#include "chrome/browser/policy/chrome_browser_policy_connector.h"
 #include "chrome/browser/policy/dm_token_utils.h"
 #include "chrome/browser/profiles/profile.h"
+#include "components/enterprise/browser/controller/browser_dm_token_storage.h"
 #include "components/enterprise/common/proto/connectors.pb.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/policy/core/common/cloud/dm_token.h"
+#include "components/policy/core/common/cloud/machine_level_user_cloud_policy_manager.h"
+#include "components/policy/core/common/cloud/user_cloud_policy_manager.h"
 #include "components/policy/core/common/policy_types.h"
 #include "components/user_prefs/user_prefs.h"
 #include "content/public/browser/browser_context.h"
@@ -23,6 +30,9 @@ namespace enterprise_connectors {
 const base::Feature kEnterpriseConnectorsEnabled{
     "EnterpriseConnectorsEnabled", base::FEATURE_ENABLED_BY_DEFAULT};
 
+const base::Feature kPerProfileConnectorsEnabled{
+    "PerProfileConnectorsEnabled", base::FEATURE_DISABLED_BY_DEFAULT};
+
 const char kServiceProviderConfig[] = R"({
   "version": "1",
   "service_providers" : [
@@ -117,7 +127,7 @@ base::Optional<ReportingSettings> ConnectorsService::GetReportingSettings(
   if (!ConnectorsEnabled())
     return base::nullopt;
 
-  base::Optional<DmToken> dm_token = GetDmToken(ConnectorPref(connector));
+  base::Optional<DmToken> dm_token = GetDmToken(ConnectorScopePref(connector));
   if (!dm_token.has_value())
     return base::nullopt;
 
@@ -125,6 +135,8 @@ base::Optional<ReportingSettings> ConnectorsService::GetReportingSettings(
       connectors_manager_->GetReportingSettings(connector);
   if (settings.has_value()) {
     settings.value().dm_token = dm_token.value().value;
+    settings.value().per_profile =
+        dm_token.value().scope == policy::POLICY_SCOPE_USER;
   }
 
   return settings;
@@ -136,7 +148,7 @@ base::Optional<AnalysisSettings> ConnectorsService::GetAnalysisSettings(
   if (!ConnectorsEnabled())
     return base::nullopt;
 
-  base::Optional<DmToken> dm_token = GetDmToken(ConnectorPref(connector));
+  base::Optional<DmToken> dm_token = GetDmToken(ConnectorScopePref(connector));
   if (!dm_token.has_value())
     return base::nullopt;
 
@@ -183,10 +195,20 @@ ConnectorsService::DmToken& ConnectorsService::DmToken::operator=(DmToken&&) =
 ConnectorsService::DmToken::~DmToken() = default;
 
 base::Optional<ConnectorsService::DmToken> ConnectorsService::GetDmToken(
-    const char* pref) {
-  // TODO(crbug.com/1148789): Add code to check the scope of |pref| and handle
-  // the "user" case.
+    const char* scope_pref) const {
+#if defined(OS_CHROMEOS)
+  // On CrOS, the device must be affiliated to use the DM token for
+  // scanning/reporting so we always use the browser DM token.
+  return GetBrowserDmToken();
+#else
+  return GetPolicyScope(scope_pref) == policy::POLICY_SCOPE_USER
+             ? GetProfileDmToken()
+             : GetBrowserDmToken();
+#endif
+}
 
+base::Optional<ConnectorsService::DmToken>
+ConnectorsService::GetBrowserDmToken() const {
   policy::DMToken dm_token =
       policy::GetDMToken(Profile::FromBrowserContext(context_));
 
@@ -196,6 +218,60 @@ base::Optional<ConnectorsService::DmToken> ConnectorsService::GetDmToken(
   return DmToken(dm_token.value(), policy::POLICY_SCOPE_MACHINE);
 }
 
+#if !defined(OS_CHROMEOS)
+base::Optional<ConnectorsService::DmToken>
+ConnectorsService::GetProfileDmToken() const {
+  if (!base::FeatureList::IsEnabled(kPerProfileConnectorsEnabled))
+    return base::nullopt;
+
+  if (!CanUseProfileDmToken())
+    return base::nullopt;
+
+  policy::UserCloudPolicyManager* policy_manager =
+      Profile::FromBrowserContext(context_)->GetUserCloudPolicyManager();
+  if (!policy_manager || !policy_manager->IsClientRegistered())
+    return base::nullopt;
+
+  return DmToken(policy_manager->core()->client()->dm_token(),
+                 policy::POLICY_SCOPE_USER);
+}
+
+bool ConnectorsService::CanUseProfileDmToken() const {
+  // If the browser isn't managed by CBCM, then the profile DM token can be
+  // used.
+  if (!policy::BrowserDMTokenStorage::Get()->RetrieveDMToken().is_valid())
+    return true;
+
+  policy::UserCloudPolicyManager* profile_policy_manager =
+      Profile::FromBrowserContext(context_)->GetUserCloudPolicyManager();
+  policy::MachineLevelUserCloudPolicyManager* browser_policy_manager =
+      g_browser_process->browser_policy_connector()
+          ->machine_level_user_cloud_policy_manager();
+
+  if (!profile_policy_manager || !browser_policy_manager ||
+      !profile_policy_manager->IsClientRegistered() ||
+      !browser_policy_manager->IsClientRegistered()) {
+    return false;
+  }
+
+  auto* profile_policy = profile_policy_manager->core()->store()->policy();
+  auto* browser_policy = browser_policy_manager->core()->store()->policy();
+
+  if (!profile_policy || !browser_policy)
+    return false;
+
+  return chrome::enterprise_util::IsProfileAffiliated(*profile_policy,
+                                                      *browser_policy);
+}
+#endif  // !defined(OS_CHROMEOS)
+
+policy::PolicyScope ConnectorsService::GetPolicyScope(
+    const char* scope_pref) const {
+  return static_cast<policy::PolicyScope>(
+      Profile::FromBrowserContext(context_)->GetPrefs()->GetInteger(
+          scope_pref));
+}
+
 bool ConnectorsService::ConnectorsEnabled() const {
   if (!base::FeatureList::IsEnabled(kEnterpriseConnectorsEnabled))
     return false;

chrome/browser/enterprise/connectors/connectors_service.h

@@ -25,6 +25,9 @@ namespace enterprise_connectors {
 // ConnectorsManager.
 extern const base::Feature kEnterpriseConnectorsEnabled;
 
+// Controls whether per-profile Enterprise Connector policies are applied.
+extern const base::Feature kPerProfileConnectorsEnabled;
+
 // For the moment, service provider configurations are static and only support
 // google endpoints.  Therefore the configuration is placed here directly.
 // Once the configuration becomes more dynamic this static string will be
@@ -71,8 +74,26 @@ class ConnectorsService : public KeyedService {
     // policy used to get a DM token.
     policy::PolicyScope scope;
   };
-  base::Optional<DmToken> GetDmToken(const char* pref);
 
+  // Returns the DM token to use with the given |scope_pref|. That pref should
+  // contain either POLICY_SCOPE_MACHINE or POLICY_SCOPE_USER.
+  base::Optional<DmToken> GetDmToken(const char* scope_pref) const;
+  base::Optional<DmToken> GetBrowserDmToken() const;
+#if !defined(OS_CHROMEOS)
+  base::Optional<DmToken> GetProfileDmToken() const;
+
+  // Returns true if the browser isn't managed by CBCM, otherwise this checks if
+  // the affiliations IDs from the profile and browser policy fetching responses
+  // indicate that the same customer manages both.
+  bool CanUseProfileDmToken() const;
+#endif
+
+  // Returns the policy::PolicyScope stored in the given |scope_pref|.
+  policy::PolicyScope GetPolicyScope(const char* scope_pref) const;
+
+  // Returns whether Connectors are enabled at all. This can be false if:
+  // - The kEnterpriseConnectorsEnabled feature is disabled
+  // - The profile is incognito
   bool ConnectorsEnabled() const;
 
   content::BrowserContext* context_;

chrome/browser/enterprise/connectors/connectors_service_unittest.cc

@@ -11,6 +11,7 @@
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "components/enterprise/common/proto/connectors.pb.h"
+#include "components/policy/core/common/policy_types.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -130,6 +131,8 @@ class ConnectorsServiceReportingFeatureTest
 
   const char* pref() const { return ConnectorPref(connector()); }
 
+  const char* scope_pref() const { return ConnectorScopePref(connector()); }
+
   const char* pref_value() const {
     switch (policy_value()) {
       case 1:
@@ -155,8 +158,11 @@ class ConnectorsServiceReportingFeatureTest
 };
 
 TEST_P(ConnectorsServiceReportingFeatureTest, Test) {
-  if (policy_value() != 0)
+  if (policy_value() != 0) {
     profile_->GetPrefs()->Set(pref(), *base::JSONReader::Read(pref_value()));
+    profile_->GetPrefs()->SetInteger(scope_pref(),
+                                     policy::POLICY_SCOPE_MACHINE);
+  }
 
   auto settings = ConnectorsServiceFactory::GetForBrowserContext(profile_)
                       ->GetReportingSettings(connector());

chrome/browser/enterprise/util/affiliation.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 "chrome/browser/enterprise/util/affiliation.h"
+#include <set>
+
+namespace chrome {
+namespace enterprise_util {
+
+bool IsProfileAffiliated(
+    const enterprise_management::PolicyData& profile_policy,
+    const enterprise_management::PolicyData& browser_policy) {
+  std::set<std::string> profile_affiliation_ids;
+  profile_affiliation_ids.insert(profile_policy.user_affiliation_ids().begin(),
+                                 profile_policy.user_affiliation_ids().end());
+  for (const std::string& browser_affiliation_id :
+       browser_policy.device_affiliation_ids()) {
+    if (profile_affiliation_ids.count(browser_affiliation_id))
+      return true;
+  }
+  return false;
+}
+
+}  // namespace enterprise_util
+}  // namespace chrome

chrome/browser/enterprise/util/affiliation.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 CHROME_BROWSER_ENTERPRISE_UTIL_AFFILIATION_H_
+#define CHROME_BROWSER_ENTERPRISE_UTIL_AFFILIATION_H_
+
+#include "device_management_backend.pb.h"
+
+namespace chrome {
+namespace enterprise_util {
+
+// Returns true if the profile and browser are managed by the same customer
+// (affiliated). This is determined by comparing affiliation IDs obtained in the
+// policy fetching response. If either policies has no affiliation IDs, this
+// function returns false.
+bool IsProfileAffiliated(
+    const enterprise_management::PolicyData& profile_policy,
+    const enterprise_management::PolicyData& browser_policy);
+
+}  // namespace enterprise_util
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_ENTERPRISE_UTIL_AFFILIATION_H_

chrome/browser/enterprise/util/affiliation_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 "chrome/browser/enterprise/util/affiliation.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chrome {
+namespace enterprise_util {
+
+namespace {
+const char kAffiliationID1[] = "id1";
+const char kAffiliationID2[] = "id2";
+}  // namespace
+
+TEST(BrowserAffiliationTest, Affiliated) {
+  enterprise_management::PolicyData profile_policy;
+  profile_policy.add_user_affiliation_ids(kAffiliationID1);
+  enterprise_management::PolicyData browser_policy;
+  browser_policy.add_device_affiliation_ids(kAffiliationID1);
+
+  EXPECT_TRUE(IsProfileAffiliated(profile_policy, browser_policy));
+}
+
+TEST(BrowserAffiliationTest, Unaffiliated) {
+  enterprise_management::PolicyData profile_policy;
+  profile_policy.add_user_affiliation_ids(kAffiliationID1);
+  enterprise_management::PolicyData browser_policy;
+  browser_policy.add_device_affiliation_ids(kAffiliationID2);
+
+  EXPECT_FALSE(IsProfileAffiliated(profile_policy, browser_policy));
+}
+
+TEST(BrowserAffiliationTest, BrowserPolicyEmpty) {
+  enterprise_management::PolicyData profile_policy;
+  profile_policy.add_user_affiliation_ids(kAffiliationID1);
+  enterprise_management::PolicyData browser_policy;
+
+  EXPECT_FALSE(IsProfileAffiliated(profile_policy, browser_policy));
+}
+
+TEST(BrowserAffiliationTest, ProfilePolicyEmpty) {
+  enterprise_management::PolicyData profile_policy;
+  enterprise_management::PolicyData browser_policy;
+  browser_policy.add_device_affiliation_ids(kAffiliationID2);
+
+  EXPECT_FALSE(IsProfileAffiliated(profile_policy, browser_policy));
+}
+
+}  // namespace enterprise_util
+}  // namespace chrome

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_browsertest_base.h

@@ -49,8 +49,10 @@ class DeepScanningBrowserTestBase : public InProcessBrowserTest {
 
   const std::vector<base::FilePath>& created_file_paths() const;
 
- private:
+ protected:
   base::test::ScopedFeatureList scoped_feature_list_;
+
+ private:
   base::RepeatingClosure quit_closure_;
   enterprise_connectors::ContentAnalysisResponse
       connector_status_callback_response_;

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_test_utils.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/extensions/api/safe_browsing_private/safe_browsing_private_event_router.h"
 #include "components/policy/core/common/cloud/mock_cloud_policy_client.h"
 #include "components/policy/core/common/cloud/realtime_reporting_job_configuration.h"
+#include "components/policy/core/common/policy_types.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "content/public/browser/browser_context.h"
@@ -343,16 +344,22 @@ void EventReportValidator::SetDoneClosure(base::RepeatingClosure closure) {
 
 void SetAnalysisConnector(PrefService* prefs,
                           enterprise_connectors::AnalysisConnector connector,
-                          const std::string& pref_value) {
+                          const std::string& pref_value,
+                          bool machine_scope) {
   ListPrefUpdate settings_list(prefs, ConnectorPref(connector));
   DCHECK(settings_list.Get());
   if (!settings_list->empty())
     settings_list->Clear();
 
   settings_list->Append(*base::JSONReader::Read(pref_value));
+  prefs->SetInteger(
+      ConnectorScopePref(connector),
+      machine_scope ? policy::POLICY_SCOPE_MACHINE : policy::POLICY_SCOPE_USER);
 }
 
-void SetOnSecurityEventReporting(PrefService* prefs, bool enabled) {
+void SetOnSecurityEventReporting(PrefService* prefs,
+                                 bool enabled,
+                                 bool machine_scope) {
   ListPrefUpdate settings_list(prefs,
                                enterprise_connectors::kOnSecurityEventPref);
   DCHECK(settings_list.Get());
@@ -364,18 +371,22 @@ void SetOnSecurityEventReporting(PrefService* prefs, bool enabled) {
                       base::Value("google"));
       settings_list->Append(std::move(settings));
     }
+    prefs->SetInteger(enterprise_connectors::kOnSecurityEventScopePref,
+                      machine_scope ? policy::POLICY_SCOPE_MACHINE
+                                    : policy::POLICY_SCOPE_USER);
   } else {
     settings_list->ClearList();
+    prefs->ClearPref(enterprise_connectors::kOnSecurityEventScopePref);
   }
 }
 
 void ClearAnalysisConnector(
     PrefService* prefs,
-
     enterprise_connectors::AnalysisConnector connector) {
   ListPrefUpdate settings_list(prefs, ConnectorPref(connector));
   DCHECK(settings_list.Get());
   settings_list->Clear();
+  prefs->ClearPref(ConnectorScopePref(connector));
 }
 
 }  // namespace safe_browsing

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_test_utils.h

@@ -148,8 +148,11 @@ class EventReportValidator {
 // Helper functions that set Connector policies for testing.
 void SetAnalysisConnector(PrefService* prefs,
                           enterprise_connectors::AnalysisConnector connector,
-                          const std::string& pref_value);
-void SetOnSecurityEventReporting(PrefService* prefs, bool enabled);
+                          const std::string& pref_value,
+                          bool machine_scope = true);
+void SetOnSecurityEventReporting(PrefService* prefs,
+                                 bool enabled,
+                                 bool machine_scope = true);
 void ClearAnalysisConnector(PrefService* prefs,
                             enterprise_connectors::AnalysisConnector connector);
 

chrome/test/BUILD.gn

@@ -1039,6 +1039,7 @@ if (!is_android) {
       "../browser/download/download_frame_policy_browsertest.cc",
       "../browser/download/download_started_animation_browsertest.cc",
       "../browser/download/save_page_browsertest.cc",
+      "../browser/enterprise/connectors/connectors_service_browsertest.cc",
       "../browser/enterprise/connectors/content_analysis_delegate_browsertest.cc",
       "../browser/enterprise/connectors/content_analysis_dialog_browsertest.cc",
       "../browser/enterprise/reporting/report_scheduler_browsertest.cc",
@@ -3496,6 +3497,7 @@ test("unit_tests") {
     "../browser/engagement/site_engagement_helper_unittest.cc",
     "../browser/engagement/site_engagement_score_unittest.cc",
     "../browser/engagement/site_engagement_service_unittest.cc",
+    "../browser/enterprise/util/affiliation_unittest.cc",
     "../browser/enterprise/util/managed_browser_utils_unittest.cc",
     "../browser/external_protocol/external_protocol_handler_unittest.cc",
     "../browser/federated_learning/floc_id_provider_unittest.cc",

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

@@ -87,4 +87,10 @@ void CloudPolicyStore::SetPolicyMapForTesting(const PolicyMap& policy_map) {
 void CloudPolicyStore::SetFirstPoliciesLoaded(bool loaded) {
   first_policies_loaded_ = loaded;
 }
+
+void CloudPolicyStore::set_policy_data_for_testing(
+    std::unique_ptr<enterprise_management::PolicyData> policy) {
+  policy_ = std::move(policy);
+}
+
 }  // namespace policy

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

@@ -161,6 +161,10 @@ class POLICY_EXPORT CloudPolicyStore {
   // Sets whether or not the first policies for this policy store were loaded.
   void SetFirstPoliciesLoaded(bool loaded);
 
+  // Test helper to set |policy_|.
+  void set_policy_data_for_testing(
+      std::unique_ptr<enterprise_management::PolicyData> policy);
+
  protected:
   // Invokes the corresponding callback on all registered observers.
   void NotifyStoreLoaded();