Handle prohibited technologies in device policy ONC

chromeos/network/shill_property_handler.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/shill_property_handler.h"
 
 #include <sstream>
 
 #include "base/bind.h"
 #include "base/format_macros.h"
 #include "base/stl_util.h"
+#include "base/strings/string_util.h"

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

not needed?

[fqj, 2015/11/12 10:23:01]

Needed.

  // Send updated prohibited technology list to shill.
  const std::string prohibited_list =
      base::JoinString(prohibited_technologies, ",");

 #include "base/values.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_device_client.h"
 #include "chromeos/dbus/shill_ipconfig_client.h"
 #include "chromeos/dbus/shill_manager_client.h"
 #include "chromeos/dbus/shill_profile_client.h"
 #include "chromeos/dbus/shill_service_client.h"
+#include "chromeos/network/managed_network_configuration_handler.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_handler_callbacks.h"

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

Not needed

[fqj, 2015/11/12 10:23:01]

Done.

 #include "chromeos/network/network_state.h"
 #include "components/device_event_log/device_event_log.h"
 #include "dbus/object_path.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace {
 
 // Limit the number of services or devices we observe. Since they are listed in
 // priority order, it should be reasonable to ignore services past this.
 const size_t kMaxObserved = 100;
@@ skipping 68 matching lines @@
   Handler handler_;
 
   DISALLOW_COPY_AND_ASSIGN(ShillPropertyObserver);
 };
 
 //------------------------------------------------------------------------------
 // ShillPropertyHandler
 
 ShillPropertyHandler::ShillPropertyHandler(Listener* listener)
     : listener_(listener),
-      shill_manager_(DBusThreadManager::Get()->GetShillManagerClient()) {
-}
+      shill_manager_(DBusThreadManager::Get()->GetShillManagerClient()) {}
 
 ShillPropertyHandler::~ShillPropertyHandler() {
   // Delete network service observers.
   STLDeleteContainerPairSecondPointers(observed_networks_.begin(),
                                        observed_networks_.end());
   STLDeleteContainerPairSecondPointers(observed_devices_.begin(),
                                        observed_devices_.end());
   CHECK(shill_manager_ == DBusThreadManager::Get()->GetShillManagerClient());
   shill_manager_->RemovePropertyChangedObserver(this);
 }
@@ skipping 27 matching lines @@
 bool ShillPropertyHandler::IsTechnologyUninitialized(
     const std::string& technology) const {
   return uninitialized_technologies_.count(technology) != 0;
 }
 
 void ShillPropertyHandler::SetTechnologyEnabled(
     const std::string& technology,
     bool enabled,
     const network_handler::ErrorCallback& error_callback) {
   if (enabled) {
+    if (prohibited_technologies_.find(technology) !=
+        prohibited_technologies_.end()) {
+      chromeos::network_handler::RunErrorCallback(
+          error_callback, "", "prohibited_technologies",
+          "Ignored: Attempt to enable prohibited network technology " +
+              technology);

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

Align

[fqj, 2015/11/12 10:23:01]

Not modified.
Actually, git cl format made it like this....
If I aligns it, presubmit gives warning to ask me to run git cl format chromeos.
And then clang-format just make it like this again..

[stevenjb, 2015/11/12 18:20:38]

Sorry, you're right, didn't notice there was a + on the prev line.

+      return;
+    }
     enabling_technologies_.insert(technology);
     shill_manager_->EnableTechnology(
         technology, base::Bind(&base::DoNothing),
         base::Bind(&ShillPropertyHandler::EnableTechnologyFailed, AsWeakPtr(),
                    technology, error_callback));
   } else {
     // Immediately clear locally from enabled and enabling lists.
     enabled_technologies_.erase(technology);
     enabling_technologies_.erase(technology);
     shill_manager_->DisableTechnology(
         technology, base::Bind(&base::DoNothing),
         base::Bind(&network_handler::ShillErrorCallbackFunction,
                    "SetTechnologyEnabled Failed", technology, error_callback));
   }
 }
 
+void ShillPropertyHandler::SetProhibitedTechnologies(
+    const std::vector<std::string>& prohibited_technologies,
+    const network_handler::ErrorCallback& error_callback) {
+  // Remove technologies from the other lists.

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

Update / move comment

[fqj, 2015/11/12 10:23:01]

Done.

+  for (const auto& technology : prohibited_technologies)
+    prohibited_technologies_.insert(technology);
+  for (auto iter_prohibited_technology = prohibited_technologies_.begin();
+       iter_prohibited_technology != prohibited_technologies_.end();) {
+    if (std::find(prohibited_technologies.begin(),
+                  prohibited_technologies.end(),
+                  *iter_prohibited_technology) == prohibited_technologies.end())
+      iter_prohibited_technology =
+          prohibited_technologies_.erase(iter_prohibited_technology);
+    else
+      iter_prohibited_technology++;

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

{}

[fqj, 2015/11/12 10:23:01]

189-197 no longer used.

+  }

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

Why not just clear prohibited_technologies_ and replace it with
prohibited_technologies?

[fqj, 2015/11/12 10:23:01]

Done.
But is it possible trying SetTechnologyEnabled being called in another thread
while manipulating prohibited_technologies_ here?

[stevenjb, 2015/11/12 18:20:38]

No. DBus calls must all be made from the UI thread, so it is important that none
of this code be called from any other thread. (There are checks in lower level
code to catch that).

+
+  for (const auto& technology : prohibited_technologies) {
+    enabling_technologies_.erase(technology);
+    enabled_technologies_.erase(technology);
+    shill_manager_->DisableTechnology(
+        technology, base::Bind(&base::DoNothing),
+        base::Bind(&network_handler::ShillErrorCallbackFunction,
+                   "DisableTechnology Failed", technology, error_callback));
+  }
+
+  // Send updated prohibited technology list to shill.
+  const std::string prohibited_list =
+      base::JoinString(prohibited_technologies, ",");
+  base::StringValue value(prohibited_list);
+  shill_manager_->SetProperty(
+      "ProhibitedTechnologies", value, base::Bind(&base::DoNothing),
+      base::Bind(&network_handler::ShillErrorCallbackFunction,
+                 "SetTechnologiesProhibited Failed", prohibited_list,
+                 error_callback));
+}
+
 void ShillPropertyHandler::SetCheckPortalList(
     const std::string& check_portal_list) {
   base::StringValue value(check_portal_list);
   shill_manager_->SetProperty(
       shill::kCheckPortalListProperty, value, base::Bind(&base::DoNothing),
       base::Bind(&network_handler::ShillErrorCallbackFunction,
                  "SetCheckPortalList Failed", "Manager",
                  network_handler::ErrorCallback()));
 }
 
@@ skipping 334 matching lines @@
     NET_LOG(EVENT) << "Failed to get IP Config properties: " << ip_config_path
                    << ": " << call_status << ", For: " << path;
     return;
   }
   NET_LOG(EVENT) << "IP Config properties received: " << path;
   listener_->UpdateIPConfigProperties(type, path, ip_config_path, properties);
 }
 
 }  // namespace internal
 }  // namespace chromeos