Implement 'sensitiveValue' for regular policies

This CL does four things:
* Support 'sensitiveValue': True for regular policies defined in
  policy_templates.json
* Improve de-duplication logic for SchemaNode generation by also
  re-using more complex schemas. (Reduces number of |SchemaNode|s from
  180 to 131)
* Fix a crash if 'sensitiveValue': True was applied to a schema with
  type 'list' or 'object' (see |CopyAndMaybeConvert()| in
  policy_conversion.cc) and added tests for this.
* Add documentation about 'sensitiveValue' to policy_templates.json.
* Make use of namedtuples in generate_policy_source.py

Bug: 905357
Change-Id: If3889ada705d9576165c2467f6dd8d9376e54886
Reviewed-on: https://chromium-review.googlesource.com/c/1337346Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613228}

chrome/browser/policy/policy_conversions.cc

@@ -88,10 +88,10 @@ Value CopyAndMaybeConvert(const Value& value,
     schema->MaskSensitiveValues(&value_copy);
   if (!convert_values)
     return value_copy;
-  if (value.is_dict())
+  if (value_copy.is_dict())
     return Value(DictionaryToJSONString(value_copy));
 
-  if (!value.is_list()) {
+  if (!value_copy.is_list()) {
     return value_copy;
   }
 

chrome/browser/ui/webui/policy_ui_browsertest.cc

@@ -548,17 +548,48 @@ IN_PROC_BROWSER_TEST_F(PolicyUITest, ExtensionLoadAndSendPolicy) {
   base::ScopedTempDir temp_dir_;
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
 
-  const std::string kNewPolicyName = "new_policy";
-  const std::string kSensitivePolicyName = "sensitive_policy";
+  const std::string kNormalBooleanPolicy = "normal_boolean";
+  const std::string kSensitiveBooleanPolicy = "sensitive_boolean";
+  const std::string kSensitiveStringPolicy = "sensitive_string";
+  const std::string kSensitiveObjectPolicy = "sensitive_object";
+  const std::string kSensitiveArrayPolicy = "sensitive_array";
+  const std::string kSensitiveIntegerPolicy = "sensitive_integer";
+  const std::string kSensitiveNumberPolicy = "sensitive_number";
   std::string json_data = R"({
     "type": "object",
     "properties": {
-      "new_policy": {
-        "type": "string"
+      "normal_boolean": {
+        "type": "boolean"
       },
-      "sensitive_policy": {
+      "sensitive_boolean": {
+        "type": "boolean",
+        "sensitiveValue": true
+      },
+      "sensitive_string": {
         "type": "string",
         "sensitiveValue": true
+      },
+      "sensitive_object": {
+        "type": "object",
+        "additionalProperties": {
+          "type": "boolean"
+        },
+        "sensitiveValue": true
+      },
+      "sensitive_array": {
+        "type": "array",
+        "items": {
+          "type": "boolean"
+        },
+        "sensitiveValue": true
+      },
+      "sensitive_integer": {
+        "type": "integer",
+        "sensitiveValue": true
+      },
+      "sensitive_number": {
+        "type": "number",
+        "sensitiveValue": true
       }
     }
   })";
@@ -606,29 +637,76 @@ IN_PROC_BROWSER_TEST_F(PolicyUITest, ExtensionLoadAndSendPolicy) {
   std::vector<std::vector<std::string>> expected_policies =
       expected_chrome_policies;
   expected_policies.push_back(PopulateExpectedPolicy(
-      kNewPolicyName, std::string(), std::string(), nullptr, false));
+      kNormalBooleanPolicy, std::string(), std::string(), nullptr, false));
+  expected_policies.push_back(PopulateExpectedPolicy(
+      kSensitiveArrayPolicy, std::string(), std::string(), nullptr, false));
+  expected_policies.push_back(PopulateExpectedPolicy(
+      kSensitiveBooleanPolicy, std::string(), std::string(), nullptr, false));
+  expected_policies.push_back(PopulateExpectedPolicy(
+      kSensitiveIntegerPolicy, std::string(), std::string(), nullptr, false));
+  expected_policies.push_back(PopulateExpectedPolicy(
+      kSensitiveNumberPolicy, std::string(), std::string(), nullptr, false));
+  expected_policies.push_back(PopulateExpectedPolicy(
+      kSensitiveObjectPolicy, std::string(), std::string(), nullptr, false));
   expected_policies.push_back(PopulateExpectedPolicy(
-      kSensitivePolicyName, std::string(), std::string(), nullptr, false));
+      kSensitiveStringPolicy, std::string(), std::string(), nullptr, false));
 
   // Verify if policy UI includes policy that extension have.
   VerifyPolicies(expected_policies);
 
+  auto object_value = std::make_unique<base::DictionaryValue>();
+  object_value->SetKey("objectProperty", base::Value(true));
+  auto array_value = std::make_unique<base::ListValue>();
+  array_value->GetList().push_back(base::Value(true));
+
   policy::PolicyMap values;
-  values.Set(kNewPolicyName, policy::POLICY_LEVEL_MANDATORY,
+  values.Set(kNormalBooleanPolicy, policy::POLICY_LEVEL_MANDATORY,
+             policy::POLICY_SCOPE_USER, policy::POLICY_SOURCE_CLOUD,
+             std::make_unique<base::Value>(true), nullptr);
+  values.Set(kSensitiveArrayPolicy, policy::POLICY_LEVEL_MANDATORY,
+             policy::POLICY_SCOPE_USER, policy::POLICY_SOURCE_CLOUD,
+             std::move(array_value), nullptr);
+  values.Set(kSensitiveBooleanPolicy, policy::POLICY_LEVEL_MANDATORY,
+             policy::POLICY_SCOPE_USER, policy::POLICY_SOURCE_CLOUD,
+             std::make_unique<base::Value>(true), nullptr);
+  values.Set(kSensitiveIntegerPolicy, policy::POLICY_LEVEL_MANDATORY,
+             policy::POLICY_SCOPE_USER, policy::POLICY_SOURCE_CLOUD,
+             std::make_unique<base::Value>(42), nullptr);
+  values.Set(kSensitiveNumberPolicy, policy::POLICY_LEVEL_MANDATORY,
              policy::POLICY_SCOPE_USER, policy::POLICY_SOURCE_CLOUD,
-             std::make_unique<base::Value>("value1"), nullptr);
-  values.Set(kSensitivePolicyName, policy::POLICY_LEVEL_MANDATORY,
+             std::make_unique<base::Value>(3.141), nullptr);
+  values.Set(kSensitiveObjectPolicy, policy::POLICY_LEVEL_MANDATORY,
              policy::POLICY_SCOPE_USER, policy::POLICY_SOURCE_CLOUD,
-             std::make_unique<base::Value>("value2"), nullptr);
+             std::move(object_value), nullptr);
+  values.Set(kSensitiveStringPolicy, policy::POLICY_LEVEL_MANDATORY,
+             policy::POLICY_SCOPE_USER, policy::POLICY_SOURCE_CLOUD,
+             std::make_unique<base::Value>("value"), nullptr);
   UpdateProviderPolicyForNamespace(extension_policy_namespace, values);
 
   // Add extension policy with values to expected policy list.
+  const std::string mask_value = "********";
   std::vector<std::vector<std::string>> expected_policies_with_values =
       expected_chrome_policies;
-  expected_policies_with_values.push_back(PopulateExpectedPolicy(
-      kNewPolicyName, "value1", "Cloud", values.Get(kNewPolicyName), false));
   expected_policies_with_values.push_back(
-      PopulateExpectedPolicy(kSensitivePolicyName, "********", "Cloud",
-                             values.Get(kSensitivePolicyName), false));
+      PopulateExpectedPolicy(kNormalBooleanPolicy, "true", "Cloud",
+                             values.Get(kNormalBooleanPolicy), false));
+  expected_policies_with_values.push_back(
+      PopulateExpectedPolicy(kSensitiveArrayPolicy, mask_value, "Cloud",
+                             values.Get(kSensitiveArrayPolicy), false));
+  expected_policies_with_values.push_back(
+      PopulateExpectedPolicy(kSensitiveBooleanPolicy, mask_value, "Cloud",
+                             values.Get(kSensitiveBooleanPolicy), false));
+  expected_policies_with_values.push_back(
+      PopulateExpectedPolicy(kSensitiveIntegerPolicy, mask_value, "Cloud",
+                             values.Get(kSensitiveIntegerPolicy), false));
+  expected_policies_with_values.push_back(
+      PopulateExpectedPolicy(kSensitiveNumberPolicy, mask_value, "Cloud",
+                             values.Get(kSensitiveNumberPolicy), false));
+  expected_policies_with_values.push_back(
+      PopulateExpectedPolicy(kSensitiveObjectPolicy, mask_value, "Cloud",
+                             values.Get(kSensitiveObjectPolicy), false));
+  expected_policies_with_values.push_back(
+      PopulateExpectedPolicy(kSensitiveStringPolicy, mask_value, "Cloud",
+                             values.Get(kSensitiveStringPolicy), false));
   VerifyPolicies(expected_policies_with_values);
 }

components/policy/core/common/schema_unittest.cc

@@ -1232,15 +1232,49 @@ TEST(SchemaTest, ItemsReference) {
 
 TEST(SchemaTest, SchemaNodeMetadataSensitiveValues) {
   std::string error;
+
+  const std::string kNormalBooleanSchema = "normal_boolean";
+  const std::string kSensitiveBooleanSchema = "sensitive_boolean";
+  const std::string kSensitiveStringSchema = "sensitive_string";
+  const std::string kSensitiveObjectSchema = "sensitive_object";
+  const std::string kSensitiveArraySchema = "sensitive_array";
+  const std::string kSensitiveIntegerSchema = "sensitive_integer";
+  const std::string kSensitiveNumberSchema = "sensitive_number";
   Schema schema = Schema::Parse(R"({
     "type": "object",
     "properties": {
-      "foo": {
+      "normal_boolean": {
         "type": "boolean"
       },
-      "bar": {
+      "sensitive_boolean": {
+        "type": "boolean",
+        "sensitiveValue": true
+      },
+      "sensitive_string": {
         "type": "string",
         "sensitiveValue": true
+      },
+      "sensitive_object": {
+        "type": "object",
+        "additionalProperties": {
+          "type": "boolean"
+        },
+        "sensitiveValue": true
+      },
+      "sensitive_array": {
+        "type": "array",
+        "items": {
+          "type": "boolean"
+        },
+        "sensitiveValue": true
+      },
+      "sensitive_integer": {
+        "type": "integer",
+        "sensitiveValue": true
+      },
+      "sensitive_number": {
+        "type": "number",
+        "sensitiveValue": true
       }
     }
   })",
@@ -1249,31 +1283,72 @@ TEST(SchemaTest, SchemaNodeMetadataSensitiveValues) {
   ASSERT_EQ(base::Value::Type::DICTIONARY, schema.type());
   EXPECT_FALSE(schema.IsSensitiveValue());
 
-  Schema foo_schema = schema.GetKnownProperty("foo");
-  ASSERT_TRUE(foo_schema.valid());
-  EXPECT_EQ(base::Value::Type::BOOLEAN, foo_schema.type());
-  EXPECT_FALSE(foo_schema.IsSensitiveValue());
-
-  Schema bar_schema = schema.GetKnownProperty("bar");
-  ASSERT_TRUE(bar_schema.valid());
-  EXPECT_EQ(base::Value::Type::STRING, bar_schema.type());
-  EXPECT_TRUE(bar_schema.IsSensitiveValue());
+  Schema normal_boolean = schema.GetKnownProperty(kNormalBooleanSchema);
+  ASSERT_TRUE(normal_boolean.valid());
+  EXPECT_EQ(base::Value::Type::BOOLEAN, normal_boolean.type());
+  EXPECT_FALSE(normal_boolean.IsSensitiveValue());
+
+  Schema sensitive_boolean = schema.GetKnownProperty(kSensitiveBooleanSchema);
+  ASSERT_TRUE(sensitive_boolean.valid());
+  EXPECT_EQ(base::Value::Type::BOOLEAN, sensitive_boolean.type());
+  EXPECT_TRUE(sensitive_boolean.IsSensitiveValue());
+
+  Schema sensitive_string = schema.GetKnownProperty(kSensitiveStringSchema);
+  ASSERT_TRUE(sensitive_string.valid());
+  EXPECT_EQ(base::Value::Type::STRING, sensitive_string.type());
+  EXPECT_TRUE(sensitive_string.IsSensitiveValue());
+
+  Schema sensitive_object = schema.GetKnownProperty(kSensitiveObjectSchema);
+  ASSERT_TRUE(sensitive_object.valid());
+  EXPECT_EQ(base::Value::Type::DICTIONARY, sensitive_object.type());
+  EXPECT_TRUE(sensitive_object.IsSensitiveValue());
+
+  Schema sensitive_array = schema.GetKnownProperty(kSensitiveArraySchema);
+  ASSERT_TRUE(sensitive_array.valid());
+  EXPECT_EQ(base::Value::Type::LIST, sensitive_array.type());
+  EXPECT_TRUE(sensitive_array.IsSensitiveValue());
+
+  Schema sensitive_integer = schema.GetKnownProperty(kSensitiveIntegerSchema);
+  ASSERT_TRUE(sensitive_integer.valid());
+  EXPECT_EQ(base::Value::Type::INTEGER, sensitive_integer.type());
+  EXPECT_TRUE(sensitive_integer.IsSensitiveValue());
+
+  Schema sensitive_number = schema.GetKnownProperty(kSensitiveNumberSchema);
+  ASSERT_TRUE(sensitive_number.valid());
+  EXPECT_EQ(base::Value::Type::DOUBLE, sensitive_number.type());
+  EXPECT_TRUE(sensitive_number.IsSensitiveValue());
 
   // Run |MaskSensitiveValues| on the top-level schema
+  base::DictionaryValue object;
+  object.SetKey("objectProperty", base::Value(true));
+  base::ListValue array;
+  array.GetList().push_back(base::Value(true));
+
   base::Value value(base::Value::Type::DICTIONARY);
-  value.SetKey("foo", base::Value(true));
-  value.SetKey("bar", base::Value("testvalue"));
+  value.SetKey(kNormalBooleanSchema, base::Value(true));
+  value.SetKey(kSensitiveBooleanSchema, base::Value(true));
+  value.SetKey(kSensitiveStringSchema, base::Value("testvalue"));
+  value.SetKey(kSensitiveObjectSchema, std::move(object));
+  value.SetKey(kSensitiveArraySchema, std::move(array));
+  value.SetKey(kSensitiveIntegerSchema, base::Value(42));
+  value.SetKey(kSensitiveNumberSchema, base::Value(3.141));
   schema.MaskSensitiveValues(&value);
 
+  base::Value value_masked("********");
   base::Value value_expected(base::Value::Type::DICTIONARY);
-  value_expected.SetKey("foo", base::Value(true));
-  value_expected.SetKey("bar", base::Value("********"));
+  value_expected.SetKey(kNormalBooleanSchema, base::Value(true));
+  value_expected.SetKey(kSensitiveBooleanSchema, value_masked.Clone());
+  value_expected.SetKey(kSensitiveStringSchema, value_masked.Clone());
+  value_expected.SetKey(kSensitiveObjectSchema, value_masked.Clone());
+  value_expected.SetKey(kSensitiveArraySchema, value_masked.Clone());
+  value_expected.SetKey(kSensitiveIntegerSchema, value_masked.Clone());
+  value_expected.SetKey(kSensitiveNumberSchema, value_masked.Clone());
   EXPECT_EQ(value_expected, value);
 
   // Run |MaskSensitiveValues| on a sub-schema
-  base::Value bar_value("testvalue");
-  bar_schema.MaskSensitiveValues(&bar_value);
-  EXPECT_EQ(base::Value("********"), bar_value);
+  base::Value string_value("testvalue");
+  sensitive_string.MaskSensitiveValues(&string_value);
+  EXPECT_EQ(value_masked.Clone(), string_value);
 }
 
 TEST(SchemaTest, SchemaNodeNoSensitiveValues) {

components/policy/resources/policy_templates.json

@@ -182,6 +182,11 @@
 #   policy itself, but also the content of the JSON strings inside the policy.
 #   Do not use this field when adding new policies.
 #
+#   In order to hide sensitive policy values from the user, you can use
+#   'sensitiveValue': True (default 'False') in the associated schema. Those
+#   values will be masked with '********' in the chrome://policy UI and policy
+#   exports. 'sensitiveValue' can be applied to all schema types.
+#
 # IDs:
 #   Since a Protocol Buffer definition is generated from this file, unique and
 #   persistent IDs for all fields (but not for groups!) are needed. These are