Don't augment GUID in ONC merging.

chromeos/network/onc/onc_merger.cc

 // Copyright (c) 2012 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/onc/onc_merger.h"
 
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/values.h"
 #include "chromeos/network/onc/onc_signature.h"
 #include "components/onc/onc_constants.h"
 
 namespace chromeos {
 namespace onc {
 namespace {
 
 typedef scoped_ptr<base::DictionaryValue> DictionaryPtr;
 
+// Returns true if the field is the identifier of a configuration, i.e. the GUID
+// of a network or a certificate. These can be special handled during merging
+// because they are always identical for the various setting sources.
+bool IsIdentifierField(const OncValueSignature& value_signature,
+                       const std::string& field_name) {
+  if (&value_signature == &kNetworkConfigurationSignature)
+    return field_name == ::onc::network_config::kGUID;
+  if (&value_signature == &kCertificateSignature)
+    return field_name == ::onc::certificate::kGUID;
+  return false;
+}
+
 // Inserts |true| at every field name in |result| that is recommended in
 // |policy|.
 void MarkRecommendedFieldnames(const base::DictionaryValue& policy,
                                base::DictionaryValue* result) {
   const base::ListValue* recommended_value = NULL;
   if (!policy.GetListWithoutPathExpansion(::onc::kRecommended,
                                           &recommended_value))
     return;
   for (base::ListValue::const_iterator it = recommended_value->begin();
        it != recommended_value->end(); ++it) {
@@ skipping 289 matching lines @@
                                                user_settings,
                                                shared_settings,
                                                active_settings);
   }
 
  protected:
   // MergeSettingsAndPolicies override.
   virtual scoped_ptr<base::Value> MergeValues(
       const std::string& key,
       const ValueParams& values) OVERRIDE {
-    scoped_ptr<base::DictionaryValue> result(new base::DictionaryValue);
+    scoped_ptr<base::DictionaryValue> augmented_value(
+        new base::DictionaryValue);
     if (values.active_setting) {
-      result->SetWithoutPathExpansion(::onc::kAugmentationActiveSetting,
-                                      values.active_setting->DeepCopy());
+      augmented_value->SetWithoutPathExpansion(
+          ::onc::kAugmentationActiveSetting, values.active_setting->DeepCopy());
     }
 
     const OncFieldSignature* field = NULL;
     if (signature_)
       field = GetFieldSignature(*signature_, key);
 
     if (field) {
       // This field is part of the provided ONCSignature, thus it can be
       // controlled by policy.
       std::string which_effective;
-      MergeToEffective::MergeValues(key, values, &which_effective).reset();
+      scoped_ptr<base::Value> effective_value =
+          MergeToEffective::MergeValues(key, values, &which_effective);
+
+      if (IsIdentifierField(*signature_, key)) {
+        // Don't augment the GUID but write the plain value.
+        DCHECK(effective_value);
+
+        // DCHECK that all provided GUIDs are identical.
+        DCHECK(!values.user_policy ||
+               effective_value->Equals(values.user_policy));
+        DCHECK(!values.device_policy ||
+               effective_value->Equals(values.device_policy));
+        DCHECK(!values.user_setting ||
+               effective_value->Equals(values.user_setting));
+        DCHECK(!values.shared_setting ||
+               effective_value->Equals(values.shared_setting));
+        DCHECK(!values.active_setting ||
+               effective_value->Equals(values.active_setting));

[stevenjb, 2014/05/27 15:38:43]

nit: Maybe wrap all of these checks into a helper and DCHECK the result.

[pneubeck (no reviews), 2014/06/03 16:21:29]

Done.

+
+        // Return the un-augmented GUID.
+        return effective_value.Pass();
+      }
+
       if (!which_effective.empty()) {
-        result->SetStringWithoutPathExpansion(
+        augmented_value->SetStringWithoutPathExpansion(
             ::onc::kAugmentationEffectiveSetting, which_effective);
       }
       bool is_credential = onc::FieldIsCredential(*signature_, key);
 
       // Prevent credentials from being forwarded in cleartext to
       // UI. User/shared credentials are not stored separately, so they cannot
       // leak here.
       if (!is_credential) {
         if (values.user_policy) {
-          result->SetWithoutPathExpansion(::onc::kAugmentationUserPolicy,
-                                          values.user_policy->DeepCopy());
+          augmented_value->SetWithoutPathExpansion(
+              ::onc::kAugmentationUserPolicy, values.user_policy->DeepCopy());
         }
         if (values.device_policy) {
-          result->SetWithoutPathExpansion(::onc::kAugmentationDevicePolicy,
-                                          values.device_policy->DeepCopy());
+          augmented_value->SetWithoutPathExpansion(
+              ::onc::kAugmentationDevicePolicy,
+              values.device_policy->DeepCopy());
         }
       }
       if (values.user_setting) {
-        result->SetWithoutPathExpansion(::onc::kAugmentationUserSetting,
-                                        values.user_setting->DeepCopy());
+        augmented_value->SetWithoutPathExpansion(
+            ::onc::kAugmentationUserSetting, values.user_setting->DeepCopy());
       }
       if (values.shared_setting) {
-        result->SetWithoutPathExpansion(::onc::kAugmentationSharedSetting,
-                                        values.shared_setting->DeepCopy());
+        augmented_value->SetWithoutPathExpansion(
+            ::onc::kAugmentationSharedSetting,
+            values.shared_setting->DeepCopy());
       }
       if (HasUserPolicy() && values.user_editable) {
-        result->SetBooleanWithoutPathExpansion(::onc::kAugmentationUserEditable,
-                                               true);
+        augmented_value->SetBooleanWithoutPathExpansion(
+            ::onc::kAugmentationUserEditable, true);
       }
       if (HasDevicePolicy() && values.device_editable) {
-        result->SetBooleanWithoutPathExpansion(
+        augmented_value->SetBooleanWithoutPathExpansion(
             ::onc::kAugmentationDeviceEditable, true);
       }
     } else {
       // This field is not part of the provided ONCSignature, thus it cannot be
       // controlled by policy.
-      result->SetStringWithoutPathExpansion(
+      augmented_value->SetStringWithoutPathExpansion(
           ::onc::kAugmentationEffectiveSetting, ::onc::kAugmentationUnmanaged);
     }
-    if (result->empty())
-      result.reset();
-    return result.PassAs<base::Value>();
+    if (augmented_value->empty())
+      augmented_value.reset();
+    return augmented_value.PassAs<base::Value>();
   }
 
   // MergeListOfDictionaries override.
   virtual DictionaryPtr MergeNestedDictionaries(
       const std::string& key,
       const DictPtrs &dicts) OVERRIDE {
     DictionaryPtr result;
     if (signature_) {
       const OncValueSignature* enclosing_signature = signature_;
       signature_ = NULL;
@@ skipping 36 matching lines @@
     const base::DictionaryValue* shared_settings,
     const base::DictionaryValue* active_settings) {
   MergeToAugmented merger;
   return merger.MergeDictionaries(
       signature, user_policy, device_policy, user_settings, shared_settings,
       active_settings);
 }
 
 }  // namespace onc
 }  // namespace chromeos