Updates PolicyLoaderIOS to support reading JSON-encoded dictionaries.

The AppConfig spec does not support complex structured data, so we will
encode "dict" policies as JSON strings.  PolicyLoaderIOS has been
updated to compare the type of the loaded policy data against the
expected type for that policy and attempt a conversion if the types
don't match.

BUG=1065906

Change-Id: Ic3ef56baba05bc7b4753c872ab8817a41d5138cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135827Reviewed-by: Owen Min <zmin@chromium.org>
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757071}

components/policy/core/common/policy_loader_ios.h

@@ -14,11 +14,14 @@
 
 namespace policy {
 
+class SchemaRegistry;
+
 // A policy loader that loads policy from the managed app configuration
 // introduced in iOS 7.
 class POLICY_EXPORT PolicyLoaderIOS : public AsyncPolicyLoader {
  public:
   explicit PolicyLoaderIOS(
+      SchemaRegistry* registry,
       scoped_refptr<base::SequencedTaskRunner> task_runner);
   ~PolicyLoaderIOS() override;
 
@@ -31,8 +34,18 @@ class POLICY_EXPORT PolicyLoaderIOS : public AsyncPolicyLoader {
   void UserDefaultsChanged();
 
   // Loads the Chrome policies in |dictionary| into the given |bundle|.
-  static void LoadNSDictionaryToPolicyBundle(NSDictionary* dictionary,
-                                             PolicyBundle* bundle);
+  void LoadNSDictionaryToPolicyBundle(NSDictionary* dictionary,
+                                      PolicyBundle* bundle);
+
+  // Validates the given policy data against the stored |schema_|, converting
+  // data to the expected type if necessary.  The returned value is suitable for
+  // adding to a PolicyMap.
+  std::unique_ptr<base::Value> ConvertPolicyDataIfNecessary(
+      const std::string& key,
+      const base::Value& value);
+
+  // The schema used by |ValidatePolicyData()|.
+  const Schema* policy_schema_;
 
   // Used to manage the registration for NSNotificationCenter notifications.
   __strong id notification_observer_;

components/policy/core/common/policy_loader_ios.mm

@@ -9,6 +9,7 @@
 #import <UIKit/UIKit.h>
 
 #include "base/bind.h"
+#include "base/json/json_reader.h"
 #include "base/location.h"
 #include "base/logging.h"
 #import "base/mac/foundation_util.h"
@@ -20,6 +21,8 @@
 #import "components/policy/core/common/policy_loader_ios_constants.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_namespace.h"
+#include "components/policy/core/common/schema.h"
+#include "components/policy/core/common/schema_registry.h"
 #include "components/policy/policy_constants.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -83,9 +86,12 @@
 namespace policy {
 
 PolicyLoaderIOS::PolicyLoaderIOS(
+    SchemaRegistry* registry,
     scoped_refptr<base::SequencedTaskRunner> task_runner)
-    : AsyncPolicyLoader(task_runner),
-      weak_factory_(this) {}
+    : AsyncPolicyLoader(task_runner), weak_factory_(this) {
+  PolicyNamespace ns(POLICY_DOMAIN_CHROME, std::string());
+  policy_schema_ = registry->schema_map()->GetSchema(ns);
+}
 
 PolicyLoaderIOS::~PolicyLoaderIOS() {
   DCHECK(task_runner()->RunsTasksInCurrentSequence());
@@ -136,7 +142,6 @@ void PolicyLoaderIOS::UserDefaultsChanged() {
   Reload(false);
 }
 
-// static
 void PolicyLoaderIOS::LoadNSDictionaryToPolicyBundle(NSDictionary* dictionary,
                                                      PolicyBundle* bundle) {
   // NSDictionary is toll-free bridged to CFDictionaryRef, which is a
@@ -150,9 +155,34 @@ void PolicyLoaderIOS::LoadNSDictionaryToPolicyBundle(NSDictionary* dictionary,
     dict->RemoveKey(base::SysNSStringToUTF8(kPolicyLoaderIOSLoadPolicyKey));
 
     PolicyMap& map = bundle->Get(PolicyNamespace(POLICY_DOMAIN_CHROME, ""));
-    map.LoadFrom(dict, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
-                 POLICY_SOURCE_PLATFORM);
+    for (const auto& it : dict->DictItems()) {
+      map.Set(it.first, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
+              POLICY_SOURCE_PLATFORM,
+              ConvertPolicyDataIfNecessary(it.first, it.second), nullptr);
+    }
+  }
+}
+
+std::unique_ptr<base::Value> PolicyLoaderIOS::ConvertPolicyDataIfNecessary(
+    const std::string& key,
+    const base::Value& value) {
+  const Schema schema = policy_schema_->GetKnownProperty(key);
+
+  if (!schema.valid()) {
+    return value.CreateDeepCopy();
+  }
+
+  // Handle the case of a JSON-encoded string for a dict policy.
+  if (schema.type() == base::Value::Type::DICTIONARY && value.is_string()) {
+    base::Optional<base::Value> decoded_value = base::JSONReader::Read(
+        value.GetString(), base::JSONParserOptions::JSON_ALLOW_TRAILING_COMMAS);
+    if (decoded_value.has_value()) {
+      return base::Value::ToUniquePtrValue(std::move(decoded_value.value()));
+    }
   }
+
+  // Otherwise return an unchanged value.
+  return value.CreateDeepCopy();
 }
 
 }  // namespace policy

components/policy/core/common/policy_loader_ios_unittest.mm

@@ -10,6 +10,7 @@
 
 #include "base/callback.h"
 #include "base/files/file_path.h"
+#include "base/json/json_string_value_serializer.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/macros.h"
 #include "base/sequenced_task_runner.h"
@@ -34,21 +35,9 @@ namespace policy {
 
 namespace {
 
-// Creates a new PolicyLoaderIOS and verifies that it does not load any
-// policies.
-void VerifyNoPoliciesAreLoaded() {
-  PolicyBundle empty;
-  scoped_refptr<base::TestSimpleTaskRunner> taskRunner =
-      new base::TestSimpleTaskRunner();
-  PolicyLoaderIOS loader(taskRunner);
-  std::unique_ptr<PolicyBundle> bundle = loader.Load();
-  ASSERT_TRUE(bundle);
-  EXPECT_TRUE(bundle->Equals(empty));
-}
-
 class TestHarness : public PolicyProviderTestHarness {
  public:
-  TestHarness();
+  TestHarness(bool encode_complex_data_as_json);
   ~TestHarness() override;
 
   void SetUp() override;
@@ -71,6 +60,7 @@ class TestHarness : public PolicyProviderTestHarness {
       const base::DictionaryValue* policy_value) override;
 
   static PolicyProviderTestHarness* Create();
+  static PolicyProviderTestHarness* CreateWithJSONEncoding();
 
  private:
   // Merges the policies in |policy| into the current policy dictionary
@@ -78,13 +68,18 @@ class TestHarness : public PolicyProviderTestHarness {
   // exists.
   void AddPolicies(NSDictionary* policy);
 
+  // If true, the test harness will encode complex data (dicts and lists) as
+  // JSON strings.
+  bool encode_complex_data_as_json_;
+
   DISALLOW_COPY_AND_ASSIGN(TestHarness);
 };
 
-TestHarness::TestHarness()
+TestHarness::TestHarness(bool encode_complex_data_as_json)
     : PolicyProviderTestHarness(POLICY_LEVEL_MANDATORY,
                                 POLICY_SCOPE_MACHINE,
-                                POLICY_SOURCE_PLATFORM) {}
+                                POLICY_SOURCE_PLATFORM),
+      encode_complex_data_as_json_(encode_complex_data_as_json) {}
 
 TestHarness::~TestHarness() {
   // Cleanup any policies left from the test.
@@ -106,7 +101,7 @@ ConfigurationPolicyProvider* TestHarness::CreateProvider(
     SchemaRegistry* registry,
     scoped_refptr<base::SequencedTaskRunner> task_runner) {
   return new AsyncPolicyProvider(
-      registry, std::make_unique<PolicyLoaderIOS>(task_runner));
+      registry, std::make_unique<PolicyLoaderIOS>(registry, task_runner));
 }
 
 void TestHarness::InstallEmptyPolicy() {
@@ -150,14 +145,29 @@ void TestHarness::InstallDictionaryPolicy(
     const std::string& policy_name,
     const base::DictionaryValue* policy_value) {
   NSString* key = base::SysUTF8ToNSString(policy_name);
-  base::ScopedCFTypeRef<CFPropertyListRef> value(
-      ValueToProperty(*policy_value));
-  AddPolicies(@{key : (__bridge NSDictionary*)(value.get())});
+
+  if (encode_complex_data_as_json_) {
+    // Convert |policy_value| to a JSON-encoded string.
+    std::string json_string;
+    JSONStringValueSerializer serializer(&json_string);
+    ASSERT_TRUE(serializer.Serialize(*policy_value));
+
+    AddPolicies(@{key : base::SysUTF8ToNSString(json_string)});
+  } else {
+    base::ScopedCFTypeRef<CFPropertyListRef> value(
+        ValueToProperty(*policy_value));
+    AddPolicies(@{key : (__bridge NSDictionary*)(value.get())});
+  }
 }
 
 // static
 PolicyProviderTestHarness* TestHarness::Create() {
-  return new TestHarness();
+  return new TestHarness(false);
+}
+
+// static
+PolicyProviderTestHarness* TestHarness::CreateWithJSONEncoding() {
+  return new TestHarness(true);
 }
 
 void TestHarness::AddPolicies(NSDictionary* policy) {
@@ -182,14 +192,60 @@ INSTANTIATE_TEST_SUITE_P(PolicyProviderIOSChromePolicyTest,
                          ConfigurationPolicyProviderTest,
                          testing::Values(TestHarness::Create));
 
-using PolicyLoaderIOSTest = PlatformTest;
+INSTANTIATE_TEST_SUITE_P(PolicyProviderIOSChromePolicyJSONTest,
+                         ConfigurationPolicyProviderTest,
+                         testing::Values(TestHarness::CreateWithJSONEncoding));
+
+class PolicyLoaderIOSTest : public PlatformTest {
+ public:
+  void SetUp() override {
+    const std::string load_policy_key =
+        base::SysNSStringToUTF8(kPolicyLoaderIOSLoadPolicyKey);
+    const std::string test_schema = "{"
+                                    "  \"type\": \"object\","
+                                    "  \"properties\": {"
+                                    "    \"" +
+                                    load_policy_key +
+                                    "\": { \"type\": \"boolean\" },"
+                                    "    \"key1\": { \"type\": \"string\" },"
+                                    "    \"key2\": { \"type\": \"string\" },"
+                                    "  }"
+                                    "}";
+
+    std::string error;
+    Schema schema = Schema::Parse(test_schema, &error);
+    ASSERT_TRUE(schema.valid());
+
+    const PolicyNamespace ns(POLICY_DOMAIN_CHROME, "");
+    schema_registry_.RegisterComponent(ns, schema);
+
+    scoped_refptr<base::TestSimpleTaskRunner> task_runner =
+        new base::TestSimpleTaskRunner();
+    loader_ = std::make_unique<PolicyLoaderIOS>(&schema_registry_, task_runner);
+  }
+
+ protected:
+  // Verifies that |loader_| does not load any policies.
+  void VerifyNoPoliciesAreLoaded() {
+    PolicyBundle empty;
+    std::unique_ptr<PolicyBundle> bundle = loader_->Load();
+    ASSERT_TRUE(bundle);
+    EXPECT_TRUE(bundle->Equals(empty));
+  }
+
+  // The schema registry used while testing.
+  SchemaRegistry schema_registry_;
+
+  // The PolicyLoaderIOS under test.
+  std::unique_ptr<PolicyLoaderIOS> loader_;
+};
 
 // Verifies that policies are not loaded if kPolicyLoaderIOSLoadPolicyKey is not
 // present.
 TEST_F(PolicyLoaderIOSTest, NoPoliciesLoadedWithoutLoadPolicyKey) {
   NSDictionary* policy = @{
-    @"shared": @"wrong",
-    @"key1": @"value1",
+    @"key1" : @"value1",
+    @"key2" : @"value2",
   };
   [[NSUserDefaults standardUserDefaults]
       setObject:policy
@@ -203,8 +259,8 @@ TEST_F(PolicyLoaderIOSTest, NoPoliciesLoadedWithoutLoadPolicyKey) {
 TEST_F(PolicyLoaderIOSTest, NoPoliciesLoadedWhenEnableChromeKeyIsFalse) {
   NSDictionary* policy = @{
     kPolicyLoaderIOSLoadPolicyKey : @NO,
-    @"shared" : @"wrong",
     @"key1" : @"value1",
+    @"key2" : @"value2",
   };
   [[NSUserDefaults standardUserDefaults]
       setObject:policy

ios/chrome/browser/policy/browser_policy_connector_ios.mm

@@ -76,9 +76,10 @@ BrowserPolicyConnectorIOS::CreatePolicyProviders() {
 
 std::unique_ptr<ConfigurationPolicyProvider>
 BrowserPolicyConnectorIOS::CreatePlatformProvider() {
-  std::unique_ptr<AsyncPolicyLoader> loader(
-      new PolicyLoaderIOS(base::ThreadPool::CreateSequencedTaskRunner(
-          {base::MayBlock(), base::TaskPriority::BEST_EFFORT})));
+  auto loader = std::make_unique<PolicyLoaderIOS>(
+      GetSchemaRegistry(),
+      base::ThreadPool::CreateSequencedTaskRunner(
+          {base::MayBlock(), base::TaskPriority::BEST_EFFORT}));
 
   return std::make_unique<AsyncPolicyProvider>(GetSchemaRegistry(),
                                                std::move(loader));