Handle prohibited technologies in device policy ONC

chromeos/network/managed_network_configuration_handler_impl.cc

Index: chromeos/network/managed_network_configuration_handler_impl.cc
diff --git a/chromeos/network/managed_network_configuration_handler_impl.cc b/chromeos/network/managed_network_configuration_handler_impl.cc
index c0407232335a56daf76c05767fa424c86380ee5c..e308f4d278144de48af30bc486a716ad4f34be9f 100644
--- a/chromeos/network/managed_network_configuration_handler_impl.cc
+++ b/chromeos/network/managed_network_configuration_handler_impl.cc
@@ -426,6 +426,28 @@ void ManagedNetworkConfigurationHandlerImpl::SetPolicy(
 
   policies->global_network_config.MergeDictionary(&global_network_config);
 
+  // Update prohibited technologies.
+  const base::ListValue* prohibited_list = nullptr;
+  if (policies->global_network_config.GetListWithoutPathExpansion(
+          ::onc::global_network_config::kDisableNetworkTypes,
+          &prohibited_list)) {
+    // Prohobited technologies are only allowed in user policy.
+    DCHECK_EQ(::onc::ONC_SOURCE_DEVICE_POLICY, onc_source);

[stevenjb, 2015/11/10 21:40:21]

Shouldn't this be checking for USER_POLICY? Also, is this enforced in the
validator? If not we shouldn't DCHECK here, just log an error.

[fqj, 2015/11/11 16:53:54]

I sent you a new PRD serveral days ago. It describes now it should be a device
policy only taking effect on user session.
ONC change to make it a device policy is also in this CL.
It's enforced in the validator.

[stevenjb, 2015/11/11 18:07:31]

I saw that, I just wanted to make sure that this isn't something that could
happen with a misconfigured ONC file.

+
+    // Build up prohibited network type list and update NetworkStateHandler.
+    std::vector<std::string> prohibited_technologies;
+    for (const base::Value* item : *prohibited_list) {
+      std::string prohibited_technology;
+      item->GetAsString(&prohibited_technology);
+      prohibited_technologies.push_back(
+          network_util::TranslateONCTypeToShill(prohibited_technology));

[stevenjb, 2015/11/10 21:40:21]

We should validate the result of TranslateONCTypeToShill to avoid passing empty
values to Shill.

[fqj, 2015/11/11 16:53:54]

Done.

+    }
+    network_state_handler_->SetProhibitedTechnologies(
+        prohibited_technologies, chromeos::network_handler::ErrorCallback());
+  }
+  if (!userhash.empty())
+    network_state_handler_->UserPolicyApplied();

[stevenjb, 2015/11/10 21:40:21]

I don't think we should need this extra call, but also see my meta comment.

[fqj, 2015/11/11 16:53:54]

Done.

+
   GuidToPolicyMap old_per_network_config;
   policies->per_network_config.swap(old_per_network_config);
 
@@ -715,6 +737,12 @@ void ManagedNetworkConfigurationHandlerImpl::Init(
   network_configuration_handler_ = network_configuration_handler;
   network_device_handler_ = network_device_handler;
   network_profile_handler_->AddObserver(this);
+
+  // Clear the list of prohibited network technologies. As a user logout always
+  // triggers a browser process restart, Init() is always invoked to reallow any
+  // network technology forbidden for the previous user.
+  network_state_handler_->SetProhibitedTechnologies(
+      std::vector<std::string>(), chromeos::network_handler::ErrorCallback());

[stevenjb, 2015/11/10 21:40:21]

This is not the right place to do this. We should handle this in NSH::Init (if
at all).

[fqj, 2015/11/11 16:53:54]

Moved to the new ProhibitedTechnologiesHandler::Init

 }
 
 void ManagedNetworkConfigurationHandlerImpl::OnPolicyAppliedToNetwork(