Extending ONC validator's logging. Completing toplevel validation.

chrome/browser/policy/configuration_policy_handler_chromeos.cc

Index: chrome/browser/policy/configuration_policy_handler_chromeos.cc
diff --git a/chrome/browser/policy/configuration_policy_handler_chromeos.cc b/chrome/browser/policy/configuration_policy_handler_chromeos.cc
index 63a6874a7ae53d4ed49533b00c6b2ed96518b364..614b18709bbf3b5ae9a7411349ce2c9b3eae33da 100644
--- a/chrome/browser/policy/configuration_policy_handler_chromeos.cc
+++ b/chrome/browser/policy/configuration_policy_handler_chromeos.cc
@@ -52,7 +52,7 @@ bool NetworkConfigurationPolicyHandler::CheckPolicySettings(
     scoped_ptr<base::DictionaryValue> root_dict =
         onc::ReadDictionaryFromJson(onc_blob);
     if (root_dict.get() == NULL) {
-      errors->AddError(policy_name(), IDS_POLICY_NETWORK_CONFIG_PARSE_ERROR);
+      errors->AddError(policy_name(), IDS_POLICY_NETWORK_CONFIG_PARSE_FAILED);
       return false;
     }
 
@@ -64,16 +64,18 @@ bool NetworkConfigurationPolicyHandler::CheckPolicySettings(
                              true);  // Validate for managed ONC
 
     // ONC policies are always unencrypted.
+    onc::Validator::Result validation_result;
     root_dict = validator.ValidateAndRepairObject(
-        &onc::kUnencryptedConfigurationSignature,
-        *root_dict);
-
-    if (root_dict.get() == NULL) {
-      errors->AddError(policy_name(), IDS_POLICY_NETWORK_CONFIG_PARSE_ERROR);
-      // Don't reject the policy, as some networks or certificates could still
-      // be applied.
-      return true;
+        &onc::kToplevelConfigurationSignature, *root_dict, &validation_result);
+    if (validation_result == onc::Validator::VALID_WITH_WARNINGS) {
+      errors->AddError(policy_name(),
+                       IDS_POLICY_NETWORK_CONFIG_VALID_WITH_WARNINGS);
+    } else if (validation_result == onc::Validator::INVALID) {
+      errors->AddError(policy_name(), IDS_POLICY_NETWORK_CONFIG_INVALID);

[Joao da Silva, 2012/12/13 09:51:35]

I'm not entirely sure if this one should return false or not. Is it possible
that INVALID is returned but some networks or certificates can still be loaded?

[pneubeck (no reviews), 2012/12/13 14:10:03]

Currently not. I apply the same validation again at the actual import
(NetworkLibrary::LoadOncNetworks) and abort there.

As long as these two validations don't get out-of-sync, a return false should be
fine.

Added return false for now, but still feeling a bit uncomfortable with that.

     }
+
+    // In any case, don't reject the policy as some networks or certificates
+    // could still be applied.
   }
 
   return true;