Extract certificate policy application from NetworkLibrary.

chromeos/network/managed_network_configuration_handler.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/network/managed_network_configuration_handler.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 781 matching lines @@
 
   std::set<std::string> remaining_policies_;
   base::WeakPtr<ManagedNetworkConfigurationHandler> handler_;
   std::string profile_path_;
 
   DISALLOW_COPY_AND_ASSIGN(PolicyApplicator);
 };
 
 void ManagedNetworkConfigurationHandler::SetPolicy(
     onc::ONCSource onc_source,
-    const base::DictionaryValue& toplevel_onc) {
+    const base::ListValue& network_configs_onc) {
   VLOG(1) << "Setting policies for ONC source "
           << onc::GetSourceAsString(onc_source) << ".";
 
-  // Validate the ONC dictionary. We are liberal and ignore unknown field
-  // names and ignore invalid field names in kRecommended arrays.
-  onc::Validator validator(false,  // Ignore unknown fields.
-                           false,  // Ignore invalid recommended field names.
-                           true,  // Fail on missing fields.
-                           true);  // This ONC comes from policy.
-  validator.SetOncSource(onc_source);
-
-  onc::Validator::Result validation_result;
-  scoped_ptr<base::DictionaryValue> onc_validated =
-      validator.ValidateAndRepairObject(
-          &onc::kToplevelConfigurationSignature,
-          toplevel_onc,
-          &validation_result);
-
-  if (validation_result == onc::Validator::VALID_WITH_WARNINGS) {
-    LOG(WARNING) << "ONC from " << onc::GetSourceAsString(onc_source)
-                 << " produced warnings.";
-  } else if (validation_result == onc::Validator::INVALID ||
-             onc_validated == NULL) {
-    LOG(ERROR) << "ONC from " << onc::GetSourceAsString(onc_source)
-               << " is invalid and couldn't be repaired.";
-    return;
-  }
-
   PolicyMap* policies;
   std::string profile;
   if (onc_source == chromeos::onc::ONC_SOURCE_USER_POLICY) {
     policies = &user_policies_by_guid_;
     profile = kUserProfilePath;
     user_policies_initialized_ = true;
   } else {
     policies = &device_policies_by_guid_;
     profile = kSharedProfilePath;
     device_policies_initialized_ = true;
   }
 
   PolicyMap old_policies;
   policies->swap(old_policies);
 
   // This stores all GUIDs of policies that have changed or are new.
   std::set<std::string> modified_policies;
 
-  base::ListValue* network_configurations = NULL;
-  onc_validated->GetListWithoutPathExpansion(
-      onc::toplevel_config::kNetworkConfigurations,
-      &network_configurations);
+  for (base::ListValue::const_iterator it = network_configs_onc.begin();
+       it != network_configs_onc.end(); ++it) {
+    const base::DictionaryValue* network = NULL;
+    (*it)->GetAsDictionary(&network);

[stevenjb, 2013/04/22 16:53:41]

Need to handle if (!GetAsDictionary()) here.

[pneubeck (no reviews), 2013/04/23 18:05:25]

The ONC must be validated before calling this function. If this call still
doesn't succeed, it will immediately segfault in the next statement.

[stevenjb, 2013/04/23 20:02:59]

nit: add DCHECK(network) to document that.

[pneubeck (no reviews), 2013/04/24 11:25:10]

Done.

+    std::string guid;
+    network->GetStringWithoutPathExpansion(onc::network_config::kGUID, &guid);

[stevenjb, 2013/04/22 16:53:41]

Not clear that the logic below will handle an empty guid correctly. You may want
to handle !GetStringWithoutPathExpansion() explicitly.

[pneubeck (no reviews), 2013/04/23 18:05:25]

Empty GUIDs are not allowed in ONC. Since this is a more implicit rule and
wouldn't lead to an immediate crash, I added a DCHECK.

 
-  if (network_configurations) {
-    while (!network_configurations->empty()) {
-      base::Value* network_value = NULL;
-      // Passes ownership of network_value.
-      network_configurations->Remove(network_configurations->GetSize() - 1,
-                                     &network_value);
-      const base::DictionaryValue* network = NULL;
-      network_value->GetAsDictionary(&network);
-      std::string guid;
-      network->GetStringWithoutPathExpansion(onc::network_config::kGUID,
-                                             &guid);
+    const base::DictionaryValue* old_entry = old_policies[guid];
+    const base::DictionaryValue*& new_entry = (*policies)[guid];
+    if (new_entry) {

[stevenjb, 2013/04/22 16:53:41]

This is headache inducing and relies on [] implicitly inserting a NULL entry.
I'd prefer something along the lines of 'if (policies->count(guid) > 0) {
LOG(ERROR)...; delete policies[guid]; }'

[pneubeck (no reviews), 2013/04/23 18:05:25]

Done.

+      LOG(ERROR) << "ONC from " << onc::GetSourceAsString(onc_source)
+                 << " contains several entries for the same GUID "
+                 << guid << ".";
+      delete new_entry;
+    }
+    new_entry = network->DeepCopy();
 
-      const base::DictionaryValue* old_entry = old_policies[guid];
-      const base::DictionaryValue*& new_entry = (*policies)[guid];
-      if (new_entry) {
-        LOG(ERROR) << "ONC from " << onc::GetSourceAsString(onc_source)
-                   << " contains several entries for the same GUID "
-                   << guid << ".";
-        delete new_entry;
-      }
-      new_entry = network;
-
-      if (!old_entry || !old_entry->Equals(new_entry)) {
-        modified_policies.insert(guid);
-      }
+    if (!old_entry || !old_entry->Equals(new_entry)) {
+      modified_policies.insert(guid);
     }

[stevenjb, 2013/04/22 16:53:41]

nit: no {}

[pneubeck (no reviews), 2013/04/23 18:05:25]

Done.

   }
 
   STLDeleteValues(&old_policies);
 
   scoped_refptr<PolicyApplicator> applicator = new PolicyApplicator(
       weak_ptr_factory_.GetWeakPtr(),
       profile,
       &modified_policies);
   applicator->Run();
 }
@@ skipping 15 matching lines @@
       device_policies_initialized_(false),
       weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
 
 ManagedNetworkConfigurationHandler::~ManagedNetworkConfigurationHandler() {
   STLDeleteValues(&user_policies_by_guid_);
   STLDeleteValues(&device_policies_by_guid_);
 }
 
 }  // namespace chromeos