Check that properties in 'required' are in the 'properties' object.

Verifiy that all 'required' properties in a policy schema are actually
existing in 'properties' as well.

Bug: 856903
Test: components_unittests --gtest_filter=JSONSchemaValidator.*
Change-Id: Ie8c3ff8ac82bcb3c436cfb580721070050b66b22
Reviewed-on: https://chromium-review.googlesource.com/1165361Reviewed-by: Lutz Justen <ljusten@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582533}

components/json_schema/json_schema_validator.cc

@@ -106,6 +106,9 @@ bool IsValidSchema(const base::DictionaryValue* dict,
   const base::DictionaryValue* dictionary_value = nullptr;
   std::string string_value;
 
+  const base::ListValue* required_properties_value = nullptr;
+  const base::DictionaryValue* properties_value = nullptr;
+
   for (base::DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
     // Validate the "type" attribute, which may be a string or a list.
     if (it.key() == schema::kType) {
@@ -195,8 +198,8 @@ bool IsValidSchema(const base::DictionaryValue* dict,
 
     // Validate the "properties" attribute. Each entry maps a key to a schema.
     if (it.key() == schema::kProperties) {
-      it.value().GetAsDictionary(&dictionary_value);
-      for (base::DictionaryValue::Iterator iter(*dictionary_value);
+      it.value().GetAsDictionary(&properties_value);
+      for (base::DictionaryValue::Iterator iter(*properties_value);
            !iter.IsAtEnd(); iter.Advance()) {
         if (!iter.value().GetAsDictionary(&dictionary_value)) {
           *error = "properties must be a dictionary";
@@ -239,14 +242,12 @@ bool IsValidSchema(const base::DictionaryValue* dict,
 
     // Validate "required" attribute.
     if (it.key() == schema::kRequired) {
-      it.value().GetAsList(&list_value);
-      for (const base::Value& value : *list_value) {
+      it.value().GetAsList(&required_properties_value);
+      for (const base::Value& value : *required_properties_value) {
         if (value.type() != base::Value::Type::STRING) {
           *error = "Invalid value in 'required' attribute";
           return false;
         }
-        // TODO(crbug.com/856903): Check that |value| is a key in
-        // schema::kProperties
       }
     }
 
@@ -296,6 +297,18 @@ bool IsValidSchema(const base::DictionaryValue* dict,
       has_type_or_ref = true;
   }
 
+  // Check that properties in'required' are in the 'properties' object.
+  if (required_properties_value) {
+    for (const base::Value& value : required_properties_value->GetList()) {
+      const std::string& name = value.GetString();
+      if (!properties_value || !properties_value->HasKey(name)) {
+        *error = "Property '" + name +
+                 "' was listed in 'required', but not defined in 'properties'.";
+        return false;
+      }
+    }
+  }
+
   if (!has_type_or_ref) {
     *error = "Schema must have a type or a $ref attribute";
     return false;

components/json_schema/json_schema_validator_unittest.cc

@@ -174,4 +174,30 @@ TEST(JSONSchemaValidator, IsValidSchema) {
       })",
                                                   0, &error))
       << error;
+
+  EXPECT_FALSE(JSONSchemaValidator::IsValidSchema(R"(
+      {
+        "type": "object",
+        "properties": {"foo": {"type": "number"}},
+        "required": ["bar"]
+      })",
+                                                  0, &error))
+      << error;
+
+  EXPECT_FALSE(JSONSchemaValidator::IsValidSchema(R"(
+      {
+        "type": "object",
+        "required": ["bar"]
+      })",
+                                                  0, &error))
+      << error;
+
+  EXPECT_TRUE(JSONSchemaValidator::IsValidSchema(R"(
+      {
+        "type": "object",
+        "properties": {"foo": {"type": "number"}},
+        "required": ["foo"]
+      })",
+                                                 0, &error))
+      << error;
 }

components/policy/tools/generate_policy_source.py

@@ -619,8 +619,6 @@ class SchemaNodesGenerator:
         self.root_properties_begin = begin
         self.root_properties_end = end
 
-      # TODO(crbug.com/856903): Check that each string in
-      # |required_properties| is in |properties|.
       required_begin = len(self.required_properties)
       required_properties = schema.get('required', [])
       assert type(required_properties) is list
@@ -628,6 +626,11 @@ class SchemaNodesGenerator:
       self.required_properties += required_properties
       required_end = len(self.required_properties)
 
+      # Check that each string in |required_properties| is in |properties|.
+      properties = schema.get('properties', {})
+      for name in required_properties:
+        assert properties.has_key(name)
+
       extra = len(self.properties_nodes)
       self.properties_nodes.append((begin, end, pattern_end, required_begin,
                                     required_end, additionalProperties, name))