Autoconnect policy for CrOS.

chromeos/network/managed_network_configuration_handler_impl.cc

 // Copyright 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_impl.h"
 
 #include <set>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 21 matching lines @@
 #include "chromeos/network/onc/onc_validator.h"
 #include "chromeos/network/policy_util.h"
 #include "chromeos/network/shill_property_util.h"
 #include "components/onc/onc_constants.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
 namespace {
 
+typedef std::map<std::string, const base::DictionaryValue*> GuidToPolicyMap;
+
 // These are error strings used for error callbacks. None of these error
 // messages are user-facing: they should only appear in logs.
 const char kInvalidUserSettingsMessage[] = "User settings are invalid.";
 const char kInvalidUserSettings[] = "Error.InvalidUserSettings";
 const char kNetworkAlreadyConfiguredMessage[] =
     "Network is already configured.";
 const char kNetworkAlreadyConfigured[] = "Error.NetworkAlreadyConfigured";
 const char kPoliciesNotInitializedMessage[] = "Policies not initialized.";
 const char kPoliciesNotInitialized[] = "Error.PoliciesNotInitialized";
 const char kProfileNotInitializedMessage[] = "Profile not initialized.";
@@ skipping 24 matching lines @@
                                            error_name,
                                            error_message)));
 }
 
 void LogErrorWithDict(const tracked_objects::Location& from_where,
                       const std::string& error_name,
                       scoped_ptr<base::DictionaryValue> error_data) {
   LOG(ERROR) << from_where.ToString() << ": " << error_name;
 }
 
-const base::DictionaryValue* GetByGUID(
-    const ManagedNetworkConfigurationHandlerImpl::GuidToPolicyMap& policies,
-    const std::string& guid) {
-  ManagedNetworkConfigurationHandlerImpl::GuidToPolicyMap::const_iterator it =
-      policies.find(guid);
+const base::DictionaryValue* GetByGUID(const GuidToPolicyMap& policies,
+                                       const std::string& guid) {
+  GuidToPolicyMap::const_iterator it = policies.find(guid);
   if (it == policies.end())
     return NULL;
   return it->second;
 }
 
 void TranslatePropertiesToOncAndRunCallback(
     const network_handler::DictionaryResultCallback& callback,
     const std::string& service_path,
     const base::DictionaryValue& shill_properties) {
   scoped_ptr<base::DictionaryValue> onc_network(
       onc::TranslateShillServiceToONCPart(
           shill_properties,
           &onc::kNetworkWithStateSignature));
   callback.Run(service_path, *onc_network);
 }
 
 }  // namespace
 
+struct ManagedNetworkConfigurationHandlerImpl::Policies {
+  ~Policies() { STLDeleteValues(&per_network_config); }

[bartfab (slow), 2013/10/16 12:40:31]

The style guide says:

"Simple accessors should generally be the only inline functions."

By my reading, that applies to structs as well, forbidding inline destructors.

[pneubeck (no reviews), 2013/10/17 10:22:26]

Hm. Whether this applies to .cc local classes as well?
Done.

[bartfab (slow), 2013/10/17 11:57:58]

Yes. There was a discussion about this on chromium-dev@ recently. Someone asked
why we have huge classes full of inline methods in tests. After some discussion,
the conclusion was that the style guide prohibits this and nobody should be
doing it. The style guide applies to all code, not just header files.

+
+  GuidToPolicyMap per_network_config;
+  base::DictionaryValue global_network_config;
+};
+
 void ManagedNetworkConfigurationHandlerImpl::AddObserver(
     NetworkPolicyObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void ManagedNetworkConfigurationHandlerImpl::RemoveObserver(
     NetworkPolicyObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
@@ skipping 60 matching lines @@
           shill_properties,
           &onc::kNetworkWithStateSignature));
 
   std::string guid;
   active_settings->GetStringWithoutPathExpansion(::onc::network_config::kGUID,
                                                  &guid);
 
   const base::DictionaryValue* user_policy = NULL;
   const base::DictionaryValue* device_policy = NULL;
   if (!guid.empty() && profile) {
-    const GuidToPolicyMap* policies = GetPoliciesForProfile(*profile);
+    const Policies* policies = GetPoliciesForProfile(*profile);
     if (!policies) {
       RunErrorCallback(service_path,
                        kPoliciesNotInitialized,
                        kPoliciesNotInitializedMessage,
                        error_callback);
       return;
     }
-    const base::DictionaryValue* policy = GetByGUID(*policies, guid);
+    const base::DictionaryValue* policy =
+        GetByGUID(policies->per_network_config, guid);
     if (profile->type() == NetworkProfile::TYPE_SHARED)
       device_policy = policy;
     else if (profile->type() == NetworkProfile::TYPE_USER)
       user_policy = policy;
     else
       NOTREACHED();
   }
 
   // This call also removes credentials from policies.
   scoped_ptr<base::DictionaryValue> augmented_properties =
@@ skipping 52 matching lines @@
     RunErrorCallback(service_path,
                      kUnknownProfilePath,
                      kUnknownProfilePathMessage,
                      error_callback);
     return;
   }
 
   VLOG(2) << "SetProperties: Found GUID " << guid << " and profile "
           << profile->ToDebugString();
 
-  const GuidToPolicyMap* policies = GetPoliciesForProfile(*profile);
+  const Policies* policies = GetPoliciesForProfile(*profile);
   if (!policies) {
     RunErrorCallback(service_path,
                      kPoliciesNotInitialized,
                      kPoliciesNotInitializedMessage,
                      error_callback);
     return;
   }
 
   // Validate the ONC dictionary. We are liberal and ignore unknown field
   // names. User settings are only partial ONC, thus we ignore missing fields.
@@ skipping 12 matching lines @@
   if (validation_result == onc::Validator::INVALID) {
     RunErrorCallback(service_path,
                      kInvalidUserSettings,
                      kInvalidUserSettingsMessage,
                      error_callback);
     return;
   }
   if (validation_result == onc::Validator::VALID_WITH_WARNINGS)
     LOG(WARNING) << "Validation of ONC user settings produced warnings.";
 
-  const base::DictionaryValue* policy = GetByGUID(*policies, guid);
+  const base::DictionaryValue* policy =
+      GetByGUID(policies->per_network_config, guid);
   VLOG(2) << "This configuration is " << (policy ? "" : "not ") << "managed.";
 
   scoped_ptr<base::DictionaryValue> shill_dictionary(
       policy_util::CreateShillConfiguration(
           *profile, guid, policy, validated_user_settings.get()));
 
   network_configuration_handler_->SetProperties(
       service_path, *shill_dictionary, callback, error_callback);
 }
 
 void ManagedNetworkConfigurationHandlerImpl::CreateConfiguration(
     const std::string& userhash,
     const base::DictionaryValue& properties,
     const network_handler::StringResultCallback& callback,
     const network_handler::ErrorCallback& error_callback) const {
-  const GuidToPolicyMap* policies = GetPoliciesForUser(userhash);
+  const Policies* policies = GetPoliciesForUser(userhash);
   if (!policies) {
     RunErrorCallback("",
                      kPoliciesNotInitialized,
                      kPoliciesNotInitializedMessage,
                      error_callback);
     return;
   }
 
-  if (policy_util::FindMatchingPolicy(*policies, properties)) {
+  if (policy_util::FindMatchingPolicy(policies->per_network_config,
+                                      properties)) {
     RunErrorCallback("",
                      kNetworkAlreadyConfigured,
                      kNetworkAlreadyConfiguredMessage,
                      error_callback);
   }
 
   const NetworkProfile* profile =
       network_profile_handler_->GetProfileForUserhash(userhash);
   if (!profile) {
     RunErrorCallback("",
@@ skipping 21 matching lines @@
     const std::string& service_path,
     const base::Closure& callback,
     const network_handler::ErrorCallback& error_callback) const {
   network_configuration_handler_->RemoveConfiguration(
       service_path, callback, error_callback);
 }
 
 void ManagedNetworkConfigurationHandlerImpl::SetPolicy(
     ::onc::ONCSource onc_source,
     const std::string& userhash,
-    const base::ListValue& network_configs_onc) {
+    const base::ListValue& network_configs_onc,
+    const base::DictionaryValue& global_network_config) {
   VLOG(1) << "Setting policies from " << ToDebugString(onc_source, userhash)
           << ".";
 
   // |userhash| must be empty for device policies.
   DCHECK(onc_source != ::onc::ONC_SOURCE_DEVICE_POLICY ||
          userhash.empty());
-  GuidToPolicyMap& policies = policies_by_user_[userhash];
+  Policies* policies = NULL;
+  if (ContainsKey(policies_by_user_, userhash)) {
+    policies = policies_by_user_[userhash].get();
+  } else {
+    policies = new Policies;
+    policies_by_user_[userhash] = make_linked_ptr(policies);
+  }
 
-  GuidToPolicyMap old_policies;
-  policies.swap(old_policies);
+  policies->global_network_config.MergeDictionary(&global_network_config);
+
+  GuidToPolicyMap old_per_network_config;
+  policies->per_network_config.swap(old_per_network_config);
 
   // This stores all GUIDs of policies that have changed or are new.
   std::set<std::string> modified_policies;
 
   for (base::ListValue::const_iterator it = network_configs_onc.begin();
        it != network_configs_onc.end(); ++it) {
     const base::DictionaryValue* network = NULL;
     (*it)->GetAsDictionary(&network);
     DCHECK(network);
 
     std::string guid;
     network->GetStringWithoutPathExpansion(::onc::network_config::kGUID, &guid);
     DCHECK(!guid.empty());
 
-    if (policies.count(guid) > 0) {
+    if (policies->per_network_config.count(guid) > 0) {
       LOG(ERROR) << "ONC from " << ToDebugString(onc_source, userhash)
                  << " contains several entries for the same GUID "
                  << guid << ".";
-      delete policies[guid];
+      delete policies->per_network_config[guid];
     }
     const base::DictionaryValue* new_entry = network->DeepCopy();
-    policies[guid] = new_entry;
+    policies->per_network_config[guid] = new_entry;
 
-    const base::DictionaryValue* old_entry = old_policies[guid];
+    const base::DictionaryValue* old_entry = old_per_network_config[guid];
     if (!old_entry || !old_entry->Equals(new_entry))
       modified_policies.insert(guid);
   }
 
-  STLDeleteValues(&old_policies);
+  STLDeleteValues(&old_per_network_config);
 
   const NetworkProfile* profile =
       network_profile_handler_->GetProfileForUserhash(userhash);
   if (!profile) {
     VLOG(1) << "The relevant Shill profile isn't initialized yet, postponing "
             << "policy application.";
     return;
   }
 
-  scoped_refptr<PolicyApplicator> applicator = new PolicyApplicator(
-      weak_ptr_factory_.GetWeakPtr(), *profile, policies, &modified_policies);
+  scoped_refptr<PolicyApplicator> applicator =
+      new PolicyApplicator(weak_ptr_factory_.GetWeakPtr(),
+                           *profile,
+                           policies->per_network_config,

[bartfab (slow), 2013/10/16 12:40:31]

Since you have a struct that groups per-network configs and global network
config, would it make sense to pass around that struct instead of its
constituent members?

[pneubeck (no reviews), 2013/10/17 10:22:26]

I think it's not worth it for two arguments only, because then the question of
where to declare that struct arises.

+                           policies->global_network_config,
+                           &modified_policies);
   applicator->Run();
 }
 
 void ManagedNetworkConfigurationHandlerImpl::OnProfileAdded(
     const NetworkProfile& profile) {
   VLOG(1) << "Adding profile " << profile.ToDebugString() << "'.";
 
-  const GuidToPolicyMap* policies = GetPoliciesForProfile(profile);
+  const Policies* policies = GetPoliciesForProfile(profile);
   if (!policies) {
     VLOG(1) << "The relevant policy is not initialized, "
             << "postponing policy application.";
     return;
   }
 
   std::set<std::string> policy_guids;
-  for (GuidToPolicyMap::const_iterator it = policies->begin();
-       it != policies->end(); ++it) {
+  for (GuidToPolicyMap::const_iterator it =
+           policies->per_network_config.begin();
+       it != policies->per_network_config.end(); ++it) {
     policy_guids.insert(it->first);
   }
 
-  scoped_refptr<PolicyApplicator> applicator = new PolicyApplicator(
-      weak_ptr_factory_.GetWeakPtr(), profile, *policies, &policy_guids);
+  scoped_refptr<PolicyApplicator> applicator =
+      new PolicyApplicator(weak_ptr_factory_.GetWeakPtr(),
+                           profile,
+                           policies->per_network_config,
+                           policies->global_network_config,
+                           &policy_guids);
   applicator->Run();
 }
 
 void ManagedNetworkConfigurationHandlerImpl::OnProfileRemoved(
     const NetworkProfile& profile) {
   // Nothing to do in this case.
 }
 
 void ManagedNetworkConfigurationHandlerImpl::CreateConfigurationFromPolicy(
     const base::DictionaryValue& shill_properties) {
   network_configuration_handler_->CreateConfiguration(
       shill_properties,
       base::Bind(&ManagedNetworkConfigurationHandlerImpl::OnPolicyApplied,
                  weak_ptr_factory_.GetWeakPtr()),
       base::Bind(&LogErrorWithDict, FROM_HERE));
 }
 
+void ManagedNetworkConfigurationHandlerImpl::
+    UpdateExistingConfigurationWithPropertiesFromPolicy(
+        const base::DictionaryValue& existing_properties,
+        const base::DictionaryValue& new_properties) {
+  base::DictionaryValue shill_properties;
+
+  std::string profile;
+  existing_properties.GetStringWithoutPathExpansion(shill::kProfileProperty,
+                                                    &profile);
+  if (profile.empty()) {
+    LOG(ERROR) << "Missing profile property.";
+    return;
+  }
+  shill_properties.SetStringWithoutPathExpansion(shill::kProfileProperty,
+                                                 profile);
+
+  if (!shill_property_util::CopyIdentifyingProperties(existing_properties,
+                                                      &shill_properties)) {
+    LOG(ERROR) << "Missing identifying properties.";
+  }
+
+  shill_properties.MergeDictionary(&new_properties);
+
+  network_configuration_handler_->CreateConfiguration(
+      shill_properties,
+      base::Bind(&ManagedNetworkConfigurationHandlerImpl::OnPolicyApplied,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&LogErrorWithDict, FROM_HERE));
+}
+
 const base::DictionaryValue*
 ManagedNetworkConfigurationHandlerImpl::FindPolicyByGUID(
     const std::string userhash,
     const std::string& guid,
     ::onc::ONCSource* onc_source) const {
   *onc_source = ::onc::ONC_SOURCE_NONE;
 
   if (!userhash.empty()) {
-    const GuidToPolicyMap* user_policies = GetPoliciesForUser(userhash);
+    const Policies* user_policies = GetPoliciesForUser(userhash);
     if (user_policies) {
-      GuidToPolicyMap::const_iterator found = user_policies->find(guid);
-      if (found != user_policies->end()) {
+      const base::DictionaryValue* policy =
+          GetByGUID(user_policies->per_network_config, guid);
+      if (policy) {
         *onc_source = ::onc::ONC_SOURCE_USER_POLICY;
-        return found->second;
+        return policy;
       }
     }
   }
 
-  const GuidToPolicyMap* device_policies = GetPoliciesForUser(std::string());
+  const Policies* device_policies = GetPoliciesForUser(std::string());
   if (device_policies) {
-    GuidToPolicyMap::const_iterator found = device_policies->find(guid);
-    if (found != device_policies->end()) {
+    const base::DictionaryValue* policy =
+        GetByGUID(device_policies->per_network_config, guid);
+    if (policy) {
       *onc_source = ::onc::ONC_SOURCE_DEVICE_POLICY;
-      return found->second;
+      return policy;
     }
   }
 
   return NULL;
 }
 
 const base::DictionaryValue*
 ManagedNetworkConfigurationHandlerImpl::FindPolicyByGuidAndProfile(
     const std::string& guid,
     const std::string& profile_path) const {
   const NetworkProfile* profile =
       network_profile_handler_->GetProfileForPath(profile_path);
   if (!profile) {
     LOG(ERROR) << "Profile path unknown: " << profile_path;
     return NULL;
   }
 
-  const GuidToPolicyMap* policies = GetPoliciesForProfile(*profile);
+  const Policies* policies = GetPoliciesForProfile(*profile);
   if (!policies)
     return NULL;
 
-  GuidToPolicyMap::const_iterator it = policies->find(guid);
-  if (it == policies->end())
-    return NULL;
-  return it->second;
+  return GetByGUID(policies->per_network_config, guid);
 }
 
-const ManagedNetworkConfigurationHandlerImpl::GuidToPolicyMap*
+const ManagedNetworkConfigurationHandlerImpl::Policies*
 ManagedNetworkConfigurationHandlerImpl::GetPoliciesForUser(
     const std::string& userhash) const {
   UserToPoliciesMap::const_iterator it = policies_by_user_.find(userhash);
   if (it == policies_by_user_.end())
     return NULL;
-  return &it->second;
+  return it->second.get();
 }
 
-const ManagedNetworkConfigurationHandlerImpl::GuidToPolicyMap*
+const ManagedNetworkConfigurationHandlerImpl::Policies*
 ManagedNetworkConfigurationHandlerImpl::GetPoliciesForProfile(
     const NetworkProfile& profile) const {
   DCHECK(profile.type() != NetworkProfile::TYPE_SHARED ||
          profile.userhash.empty());
   return GetPoliciesForUser(profile.userhash);
 }
 
 ManagedNetworkConfigurationHandlerImpl::ManagedNetworkConfigurationHandlerImpl()
     : network_state_handler_(NULL),
       network_profile_handler_(NULL),
       network_configuration_handler_(NULL),
       weak_ptr_factory_(this) {}
 
 ManagedNetworkConfigurationHandlerImpl::
     ~ManagedNetworkConfigurationHandlerImpl() {
   network_profile_handler_->RemoveObserver(this);
-  for (UserToPoliciesMap::iterator it = policies_by_user_.begin();
-       it != policies_by_user_.end(); ++it) {
-    STLDeleteValues(&it->second);
-  }
 }
 
 void ManagedNetworkConfigurationHandlerImpl::Init(
     NetworkStateHandler* network_state_handler,
     NetworkProfileHandler* network_profile_handler,
     NetworkConfigurationHandler* network_configuration_handler) {
   network_state_handler_ = network_state_handler;
   network_profile_handler_ = network_profile_handler;
   network_configuration_handler_ = network_configuration_handler;
   network_profile_handler_->AddObserver(this);
 }
 
 void ManagedNetworkConfigurationHandlerImpl::OnPolicyApplied(
     const std::string& service_path) {
   if (service_path.empty())
     return;
   FOR_EACH_OBSERVER(
       NetworkPolicyObserver, observers_, PolicyApplied(service_path));
 }
 
 }  // namespace chromeos