Add unittest for ProfilePolicyConnector proxied policies handling

Add a unittest ensuring that ProfilePolicyConnector sets up proxying of
primary user policies to the device-wide policy service.

Bug: 982936
Test: new unit_test
Change-Id: I3e19b9fa946f553d5e1df9cf9f131256545570d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1807303
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700882}

chrome/browser/policy/profile_policy_connector.cc

@@ -133,10 +133,12 @@ void ProfilePolicyConnector::Init(
   DCHECK_EQ(nullptr, user);
 #endif
 
-  if (connector->GetPlatformProvider()) {
-    wrapped_platform_policy_provider_.reset(
-        new SchemaRegistryTrackingPolicyProvider(
-            connector->GetPlatformProvider()));
+  ConfigurationPolicyProvider* platform_provider =
+      GetPlatformProvider(connector);
+  if (platform_provider) {
+    wrapped_platform_policy_provider_ =
+        std::make_unique<SchemaRegistryTrackingPolicyProvider>(
+            platform_provider);
     wrapped_platform_policy_provider_->Init(schema_registry);
     policy_providers_.push_back(wrapped_platform_policy_provider_.get());
   }
@@ -155,7 +157,7 @@ void ProfilePolicyConnector::Init(
     // Skip the platform provider since it was already handled above.  The
     // platform provider should be first in the list so that it always takes
     // precedence.
-    if (provider == connector->GetPlatformProvider()) {
+    if (provider == platform_provider) {
       continue;
     } else {
       // TODO(zmin): In the future, we may want to have special handling for
@@ -233,6 +235,11 @@ void ProfilePolicyConnector::OverrideIsManagedForTesting(bool is_managed) {
   is_managed_override_.reset(new bool(is_managed));
 }
 
+void ProfilePolicyConnector::SetPlatformPolicyProviderForTesting(
+    ConfigurationPolicyProvider* platform_policy_provider_for_testing) {
+  platform_policy_provider_for_testing_ = platform_policy_provider_for_testing;
+}
+
 void ProfilePolicyConnector::Shutdown() {
 #if defined(OS_CHROMEOS)
   if (is_primary_user_)
@@ -291,6 +298,13 @@ ProfilePolicyConnector::DeterminePolicyProviderForPolicy(
   return nullptr;
 }
 
+ConfigurationPolicyProvider* ProfilePolicyConnector::GetPlatformProvider(
+    policy::ChromeBrowserPolicyConnector* browser_policy_connector) {
+  if (platform_policy_provider_for_testing_)
+    return platform_policy_provider_for_testing_;
+  return browser_policy_connector->GetPlatformProvider();
+}
+
 #if defined(OS_CHROMEOS)
 std::unique_ptr<PolicyService>
 ProfilePolicyConnector::CreatePolicyServiceWithInitializationThrottled(

chrome/browser/policy/profile_policy_connector.h

@@ -54,6 +54,8 @@ class ProfilePolicyConnector final {
 
   void InitForTesting(std::unique_ptr<PolicyService> service);
   void OverrideIsManagedForTesting(bool is_managed);
+  void SetPlatformPolicyProviderForTesting(
+      ConfigurationPolicyProvider* platform_policy_provider_for_testing);
 
   void Shutdown();
 
@@ -80,6 +82,12 @@ class ProfilePolicyConnector final {
   const ConfigurationPolicyProvider* DeterminePolicyProviderForPolicy(
       const char* policy_key) const;
 
+  // Returns the platform policy provider, which will be used as the highest
+  // priority policy provider in PolicyService created by this
+  // ProfilePolicyConnector.
+  ConfigurationPolicyProvider* GetPlatformProvider(
+      policy::ChromeBrowserPolicyConnector* browser_policy_connector);
+
 #if defined(OS_CHROMEOS)
   // On Chrome OS, primary Profile user policies are forwarded to the
   // device-global PolicyService[1] using a ProxyPolicyProvider.
@@ -132,6 +140,10 @@ class ProfilePolicyConnector final {
   const ConfigurationPolicyProvider* configuration_policy_provider_ = nullptr;
   const CloudPolicyStore* policy_store_ = nullptr;
 
+  // If this is not nullptr, this provider will be used as (highest priority)
+  // platform policy provider.
+  ConfigurationPolicyProvider* platform_policy_provider_for_testing_ = nullptr;
+
   // |policy_providers_| contains a list of the policy providers available for
   // the PolicyService of this connector, in decreasing order of priority.
   //

chrome/browser/policy/profile_policy_connector_unittest.cc

@@ -39,6 +39,53 @@ using testing::Return;
 using testing::_;
 
 namespace policy {
+namespace {
+
+// Waits for a PolicyService to notify its observers that initialization of a
+// PolicyDomain has finished.
+class PolicyServiceInitializedWaiter : PolicyService::Observer {
+ public:
+  // Instantiates a PolicyServiceInitializedWaiter which will wait for
+  // |policy_service| to signal that |policy_domain| has completed
+  // initialization. |policy_service| must outlive this object.
+  PolicyServiceInitializedWaiter(PolicyService* policy_service,
+                                 PolicyDomain policy_domain)
+      : policy_service_(policy_service), policy_domain_(policy_domain) {
+    policy_service_->AddObserver(policy_domain_, this);
+  }
+
+  ~PolicyServiceInitializedWaiter() override {
+    policy_service_->RemoveObserver(policy_domain_, this);
+  }
+
+  // Waits for the PolicyService to signal that the PolicyDomain has completed
+  // initialization. If initialization of the PolicyDomain is already complete
+  // at the time Wait() is called, returns immediately.
+  void Wait() {
+    if (policy_service_->IsInitializationComplete(policy_domain_))
+      return;
+    run_loop_.Run();
+  }
+
+  // PolicyService::Observer:
+  void OnPolicyUpdated(const PolicyNamespace& ns,
+                       const PolicyMap& previous,
+                       const PolicyMap& current) override {}
+
+  // PolicyService::Observer:
+  void OnPolicyServiceInitialized(PolicyDomain domain) override {
+    run_loop_.Quit();
+  }
+
+ private:
+  PolicyService* policy_service_;
+  PolicyDomain policy_domain_;
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(PolicyServiceInitializedWaiter);
+};
+
+}  // namespace
 
 class ProfilePolicyConnectorTest : public testing::Test {
  protected:
@@ -46,17 +93,11 @@ class ProfilePolicyConnectorTest : public testing::Test {
   ~ProfilePolicyConnectorTest() override {}
 
   void SetUp() override {
-    // This must be set up before the TestingBrowserProcess is created.
-    BrowserPolicyConnector::SetPolicyProviderForTesting(&mock_provider_);
-
-    EXPECT_CALL(mock_provider_, IsInitializationComplete(_))
-        .WillRepeatedly(Return(true));
-
     cloud_policy_store_.NotifyStoreLoaded();
     const auto task_runner = task_environment_.GetMainThreadTaskRunner();
-    cloud_policy_manager_.reset(new CloudPolicyManager(
+    cloud_policy_manager_ = std::make_unique<CloudPolicyManager>(
         std::string(), std::string(), &cloud_policy_store_, task_runner,
-        network::TestNetworkConnectionTracker::CreateGetter()));
+        network::TestNetworkConnectionTracker::CreateGetter());
     cloud_policy_manager_->Init(&schema_registry_);
   }
 
@@ -75,7 +116,6 @@ class ProfilePolicyConnectorTest : public testing::Test {
   // Needs to be the first member.
   base::test::TaskEnvironment task_environment_;
   SchemaRegistry schema_registry_;
-  MockConfigurationPolicyProvider mock_provider_;
   MockCloudPolicyStore cloud_policy_store_;
   std::unique_ptr<CloudPolicyManager> cloud_policy_manager_;
 
@@ -91,7 +131,8 @@ TEST_F(ProfilePolicyConnectorTest, IsManagedForManagedUsers) {
                  g_browser_process->browser_policy_connector(), false);
   EXPECT_FALSE(connector.IsManaged());
 
-  cloud_policy_store_.policy_.reset(new enterprise_management::PolicyData());
+  cloud_policy_store_.policy_ =
+      std::make_unique<enterprise_management::PolicyData>();
   cloud_policy_store_.policy_->set_username("test@testdomain.com");
   cloud_policy_store_.policy_->set_state(
       enterprise_management::PolicyData::ACTIVE);
@@ -112,7 +153,8 @@ TEST_F(ProfilePolicyConnectorTest, IsManagedForActiveDirectoryUsers) {
   connector.Init(user.get(), &schema_registry_, cloud_policy_manager_.get(),
                  &cloud_policy_store_,
                  g_browser_process->browser_policy_connector(), false);
-  cloud_policy_store_.policy_.reset(new enterprise_management::PolicyData());
+  cloud_policy_store_.policy_ =
+      std::make_unique<enterprise_management::PolicyData>();
   cloud_policy_store_.policy_->set_state(
       enterprise_management::PolicyData::ACTIVE);
   EXPECT_TRUE(connector.IsManaged());
@@ -125,10 +167,67 @@ TEST_F(ProfilePolicyConnectorTest, IsManagedForActiveDirectoryUsers) {
   // Cleanup.
   connector.Shutdown();
 }
+
+TEST_F(ProfilePolicyConnectorTest, PrimaryUserPoliciesProxied) {
+  auto user_manager_unique_ptr =
+      std::make_unique<chromeos::FakeChromeUserManager>();
+  chromeos::FakeChromeUserManager* user_manager = user_manager_unique_ptr.get();
+  user_manager::ScopedUserManager scoped_user_manager_enabler(
+      std::move(user_manager_unique_ptr));
+
+  cloud_policy_store_.policy_ =
+      std::make_unique<enterprise_management::PolicyData>();
+  cloud_policy_store_.policy_->set_state(
+      enterprise_management::PolicyData::ACTIVE);
+  cloud_policy_store_.policy_map_.Set(
+      key::kAutofillAddressEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+      POLICY_SOURCE_CLOUD, std::make_unique<base::Value>(false), nullptr);
+  cloud_policy_store_.NotifyStoreLoaded();
+  base::RunLoop().RunUntilIdle();
+
+  ProfilePolicyConnector connector;
+  const AccountId account_id =
+      AccountId::AdFromUserEmailObjGuid("user@realm.example", "obj-guid");
+  user_manager::User* user = user_manager->AddUser(account_id);
+  user_manager->LoginUser(account_id);
+  EXPECT_EQ(user, user_manager::UserManager::Get()->GetPrimaryUser());
+  connector.Init(user, &schema_registry_, cloud_policy_manager_.get(),
+                 &cloud_policy_store_,
+                 g_browser_process->browser_policy_connector(), false);
+  EXPECT_TRUE(connector.IsManaged());
+
+  EXPECT_FALSE(connector.policy_service()->IsInitializationComplete(
+      POLICY_DOMAIN_CHROME));
+
+  PolicyServiceInitializedWaiter(connector.policy_service(),
+                                 POLICY_DOMAIN_CHROME)
+      .Wait();
+
+  PolicyNamespace chrome_ns(POLICY_DOMAIN_CHROME, std::string());
+  const base::Value* profile_policy_value =
+      connector.policy_service()->GetPolicies(chrome_ns).GetValue(
+          key::kAutofillAddressEnabled);
+  ASSERT_TRUE(profile_policy_value);
+  EXPECT_FALSE(profile_policy_value->GetBool());
+
+  const base::Value* proxied_policy_value =
+      g_browser_process->policy_service()->GetPolicies(chrome_ns).GetValue(
+          key::kAutofillAddressEnabled);
+  ASSERT_TRUE(proxied_policy_value);
+  EXPECT_FALSE(proxied_policy_value->GetBool());
+
+  // Cleanup.
+  connector.Shutdown();
+}
 #endif  // defined(OS_CHROMEOS)
 
 TEST_F(ProfilePolicyConnectorTest, IsProfilePolicy) {
+  MockConfigurationPolicyProvider mock_platform_provider;
+  EXPECT_CALL(mock_platform_provider, IsInitializationComplete(_))
+      .WillRepeatedly(Return(true));
+
   ProfilePolicyConnector connector;
+  connector.SetPlatformPolicyProviderForTesting(&mock_platform_provider);
   connector.Init(nullptr /* user */, &schema_registry_,
                  cloud_policy_manager_.get(), &cloud_policy_store_,
                  g_browser_process->browser_policy_connector(), false);
@@ -158,7 +257,7 @@ TEST_F(ProfilePolicyConnectorTest, IsProfilePolicy) {
   map.Set(key::kAutofillAddressEnabled, POLICY_LEVEL_MANDATORY,
           POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD,
           std::make_unique<base::Value>(true), nullptr);
-  mock_provider_.UpdateChromePolicy(map);
+  mock_platform_provider.UpdateChromePolicy(map);
   EXPECT_FALSE(connector.IsProfilePolicy(key::kAutofillAddressEnabled));
   value = connector.policy_service()->GetPolicies(chrome_ns).GetValue(
       key::kAutofillAddressEnabled);