Proper management for policy-configured networks.

This makes sure policy-configured networks get assigned to the correct profile. Also, they're now removed from the profile when the policy doesn't specify them any longer. Relies on the GUID fields in the network specifications, and requires https://gerrit.chromium.org/gerrit/#change,12437 on the flimflam side.

BUG=chromium-os:19411
TEST=Configure a network through policy. Remove that network and reload policy. The network should disappear from the network list.

Review URL: http://codereview.chromium.org/8804021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114623 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/cros/network_library.cc

@@ -18,6 +18,7 @@
 #include "base/i18n/icu_encoding_detection.h"
 #include "base/i18n/icu_string_conversions.h"
 #include "base/i18n/time_formatting.h"
+#include "base/memory/scoped_vector.h"
 #include "base/json/json_writer.h"  // for debug output only.
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
@@ -163,6 +164,20 @@ void ValidateUTF8(const std::string& str, std::string* output) {
   }
 }
 
+NetworkProfileType GetProfileTypeForSource(NetworkUIData::ONCSource source) {
+  switch (source) {
+    case NetworkUIData::ONC_SOURCE_DEVICE_POLICY:
+      return PROFILE_SHARED;
+    case NetworkUIData::ONC_SOURCE_USER_POLICY:
+      return PROFILE_USER;
+    case NetworkUIData::ONC_SOURCE_NONE:
+    case NetworkUIData::ONC_SOURCE_USER_IMPORT:
+      return PROFILE_NONE;
+  }
+  NOTREACHED() << "Unknown ONC source " << source;
+  return PROFILE_NONE;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // glib
 
@@ -2857,16 +2872,29 @@ bool NetworkLibraryImplBase::LoadOncNetworks(const std::string& onc_blob,
     }
   }
 
+  // Parse all networks. Bail out if that fails.
+  ScopedVector<Network> networks;
   for (int i = 0; i < parser.GetNetworkConfigsSize(); i++) {
     // Parse Open Network Configuration blob into a temporary Network object.
-    scoped_ptr<Network> network(parser.ParseNetwork(i));
-    if (!network.get()) {
+    Network* network = parser.ParseNetwork(i);
+    if (!network) {
       DLOG(WARNING) << "Cannot parse network in ONC file";
       if (error)
         *error = parser.parse_error();
       return false;
     }
-
+    networks.push_back(network);
+  }
+
+  // Configure the networks. While doing so, collect unique identifiers of the
+  // networks that are defined in the ONC blob in |network_ids|. They're later
+  // used to clean out any previously-existing networks that had been configured
+  // through policy but are no longer specified in the updated ONC blob.
+  std::set<std::string> network_ids;
+  std::string profile_path(GetProfilePath(GetProfileTypeForSource(source)));
+  for (std::vector<Network*>::iterator iter(networks.begin());
+       iter != networks.end(); ++iter) {
+    Network* network = *iter;
     DictionaryValue dict;
     for (Network::PropertyMap::const_iterator props =
              network->property_map_.begin();
@@ -2879,7 +2907,40 @@ bool NetworkLibraryImplBase::LoadOncNetworks(const std::string& onc_blob,
         VLOG(2) << "Property " << props->first << " will not be sent";
     }
 
+    // Set the appropriate profile for |source|.
+    if (!profile_path.empty())
+      dict.SetString(flimflam::kProfileProperty, profile_path);
+
     CallConfigureService(network->unique_id(), &dict);
+    network_ids.insert(network->unique_id());
+  }
+
+  // Go through the list of existing remembered networks and clean out the ones
+  // that no longer have a definition in the ONC blob. We first collect the
+  // networks and do the actual deletion later because ForgetNetwork() changes
+  // the remembered network vectors.
+  if (source != NetworkUIData::ONC_SOURCE_USER_IMPORT) {
+    std::vector<std::string> to_be_deleted;
+    for (WifiNetworkVector::iterator i(remembered_wifi_networks_.begin());
+         i != remembered_wifi_networks_.end(); ++i) {
+      WifiNetwork* network = *i;
+      if (NetworkUIData::GetONCSource(network) == source &&
+          network_ids.find(network->unique_id()) == network_ids.end())
+        to_be_deleted.push_back(network->service_path());
+    }
+
+    for (VirtualNetworkVector::iterator i(remembered_virtual_networks_.begin());
+         i != remembered_virtual_networks_.end(); ++i) {
+      VirtualNetwork* network = *i;
+      if (NetworkUIData::GetONCSource(network) == source &&
+          network_ids.find(network->unique_id()) == network_ids.end())
+        to_be_deleted.push_back(network->service_path());
+    }
+
+    for (std::vector<std::string>::const_iterator i(to_be_deleted.begin());
+         i != to_be_deleted.end(); ++i) {
+      ForgetNetwork(*i);
+    }
   }
 
   if (parser.GetNetworkConfigsSize() != 0 ||

chrome/browser/policy/network_configuration_updater.cc

@@ -10,6 +10,9 @@
 
 namespace policy {
 
+const char NetworkConfigurationUpdater::kEmptyConfiguration[] =
+    "{NetworkConfigurations:[],Certificates:[]}";
+
 NetworkConfigurationUpdater::NetworkConfigurationUpdater(
     ConfigurationPolicyProvider* provider,
     chromeos::NetworkLibrary* network_library)
@@ -57,6 +60,12 @@ void NetworkConfigurationUpdater::ApplyNetworkConfiguration(
       LOG(WARNING) << "Invalid network configuration.";
   }
 
+  // We need to load an empty configuration to get rid of any configuration
+  // that has been installed previously. An empty string also works, but
+  // generates warnings and errors, which we'd like to avoid.
+  if (new_network_config.empty())
+    new_network_config = kEmptyConfiguration;
+
   if (*cached_value != new_network_config) {
     *cached_value = new_network_config;
     std::string error;

chrome/browser/policy/network_configuration_updater.h

@@ -31,6 +31,9 @@ class NetworkConfigurationUpdater
   // ConfigurationPolicyProvider::Observer:
   virtual void OnUpdatePolicy(ConfigurationPolicyProvider* provider) OVERRIDE;
 
+  // Empty network configuration blob.
+  static const char kEmptyConfiguration[];
+
  private:
   // Grabs network configuration from policy and applies it.
   void Update();

chrome/browser/policy/network_configuration_updater_unittest.cc

@@ -20,6 +20,24 @@ static const char kFakeONC[] = "{ \"GUID\": \"1234\" }";
 class NetworkConfigurationUpdaterTest
     : public testing::TestWithParam<ConfigurationPolicyType> {
  protected:
+  virtual void SetUp() OVERRIDE {
+    EXPECT_CALL(network_library_, LoadOncNetworks(_, "", _, _))
+        .WillRepeatedly(Return(true));
+  }
+
+  // Maps configuration policy type to corresponding ONC source.
+  static chromeos::NetworkUIData::ONCSource TypeToONCSource(
+      ConfigurationPolicyType type) {
+    switch (type) {
+      case kPolicyDeviceOpenNetworkConfiguration:
+        return chromeos::NetworkUIData::ONC_SOURCE_DEVICE_POLICY;
+      case kPolicyOpenNetworkConfiguration:
+        return chromeos::NetworkUIData::ONC_SOURCE_USER_POLICY;
+      default:
+        return chromeos::NetworkUIData::ONC_SOURCE_NONE;
+    }
+  }
+
   chromeos::MockNetworkLibrary network_library_;
   MockConfigurationPolicyProvider provider_;
 };
@@ -27,8 +45,9 @@ class NetworkConfigurationUpdaterTest
 TEST_P(NetworkConfigurationUpdaterTest, InitialUpdate) {
   provider_.AddPolicy(GetParam(), Value::CreateStringValue(kFakeONC));
 
-  EXPECT_CALL(network_library_, LoadOncNetworks(kFakeONC, "", _, _))
-      .WillRepeatedly(Return(true));
+  EXPECT_CALL(network_library_,
+              LoadOncNetworks(kFakeONC, "", TypeToONCSource(GetParam()), _))
+      .WillOnce(Return(true));
 
   NetworkConfigurationUpdater updater(&provider_, &network_library_);
   Mock::VerifyAndClearExpectations(&network_library_);
@@ -38,20 +57,24 @@ TEST_P(NetworkConfigurationUpdaterTest, PolicyChange) {
   NetworkConfigurationUpdater updater(&provider_, &network_library_);
 
   // We should update if policy changes.
-  EXPECT_CALL(network_library_, LoadOncNetworks(kFakeONC, "", _, _))
+  EXPECT_CALL(network_library_,
+              LoadOncNetworks(kFakeONC, "", TypeToONCSource(GetParam()), _))
       .WillOnce(Return(true));
   provider_.AddPolicy(GetParam(), Value::CreateStringValue(kFakeONC));
   provider_.NotifyPolicyUpdated();
   Mock::VerifyAndClearExpectations(&network_library_);
 
   // No update if the set the same value again.
-  EXPECT_CALL(network_library_, LoadOncNetworks(kFakeONC, "", _, _))
+  EXPECT_CALL(network_library_,
+              LoadOncNetworks(kFakeONC, "", TypeToONCSource(GetParam()), _))
       .Times(0);
   provider_.NotifyPolicyUpdated();
   Mock::VerifyAndClearExpectations(&network_library_);
 
   // Another update is expected if the policy goes away.
-  EXPECT_CALL(network_library_, LoadOncNetworks("", "", _, _))
+  EXPECT_CALL(network_library_,
+              LoadOncNetworks(NetworkConfigurationUpdater::kEmptyConfiguration,
+                              "", TypeToONCSource(GetParam()), _))
       .WillOnce(Return(true));
   provider_.RemovePolicy(GetParam());
   provider_.NotifyPolicyUpdated();