Handle prohibited technologies in device policy ONC

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 <vector>
 
 #include "base/bind.h"
 #include "base/guid.h"
@@ skipping 408 matching lines @@
   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);
   }
 
   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);
 
   // 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);
@@ skipping 269 matching lines @@
 void ManagedNetworkConfigurationHandlerImpl::Init(
     NetworkStateHandler* network_state_handler,
     NetworkProfileHandler* network_profile_handler,
     NetworkConfigurationHandler* network_configuration_handler,
     NetworkDeviceHandler* network_device_handler) {
   network_state_handler_ = network_state_handler;
   network_profile_handler_ = network_profile_handler;
   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(
     const std::string& service_path) {
   if (service_path.empty())
     return;
   FOR_EACH_OBSERVER(
       NetworkPolicyObserver, observers_, PolicyAppliedToNetwork(service_path));
 }
 
@@ skipping 115 matching lines @@
     scoped_ptr<base::DictionaryValue> network_properties,
     GetDevicePropertiesCallback send_callback,
     const std::string& error_name,
     scoped_ptr<base::DictionaryValue> error_data) {
   NET_LOG_ERROR("Error getting device properties", service_path);
   send_callback.Run(service_path, network_properties.Pass());
 }
 
 
 }  // namespace chromeos