Support Managed NetworkState format dictionaries for controlled settings.

chrome/browser/ui/webui/options/chromeos/internet_options_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 "chrome/browser/ui/webui/options/chromeos/internet_options_handler.h"
 
 #include <ctype.h>
 
 #include <map>
 #include <string>
@@ skipping 108 matching lines @@
 const char kSetPreferNetworkMessage[] = "setPreferNetwork";
 const char kSetServerHostname[] = "setServerHostname";
 const char kSetSimCardLockMessage[] = "setSimCardLock";
 const char kShowMorePlanInfoMessage[] = "showMorePlanInfo";
 
 // These are strings used to communicate with JavaScript.
 const char kTagActivate[] = "activate";
 const char kTagActivationState[] = "activationState";
 const char kTagAddConnection[] = "add";
 const char kTagApn[] = "apn";
-const char kTagAutoConnect[] = "autoConnect";
 const char kTagCarrierSelectFlag[] = "showCarrierSelect";
 const char kTagCarrierUrl[] = "carrierUrl";
 const char kTagCellularAvailable[] = "cellularAvailable";
 const char kTagCellularEnabled[] = "cellularEnabled";
 const char kTagCellularSupportsScan[] = "cellularSupportsScan";
 const char kTagConfigure[] = "configure";
 const char kTagConnect[] = "connect";
 const char kTagControlledBy[] = "controlledBy";
 const char kTagDeviceConnected[] = "deviceConnected";
 const char kTagDisconnect[] = "disconnect";
-const char kTagErrorState[] = "errorState";
+const char kTagErrorMessage[] = "errorMessage";
 const char kTagForget[] = "forget";
-const char kTagIdentity[] = "identity";
 const char kTagLanguage[] = "language";
 const char kTagLastGoodApn[] = "lastGoodApn";
 const char kTagLocalizedName[] = "localizedName";
 const char kTagName[] = "name";
 const char kTagNameServersGoogle[] = "nameServersGoogle";
 const char kTagNameServerType[] = "nameServerType";
 const char kTagNetworkId[] = "networkId";
 const char kTagOptions[] = "options";
 const char kTagPassword[] = "password";
 const char kTagPolicy[] = "policy";
 const char kTagPreferred[] = "preferred";
 const char kTagProviderType[] = "providerType";
 const char kTagProviderApnList[] = "providerApnList";
 const char kTagRecommended[] = "recommended";
 const char kTagRecommendedValue[] = "recommendedValue";
 const char kTagRemembered[] = "remembered";
 const char kTagRememberedList[] = "rememberedList";
 const char kTagRestrictedPool[] = "restrictedPool";
 const char kTagRoamingState[] = "roamingState";
 const char kTagServerHostname[] = "serverHostname";
 const char kTagCarriers[] = "carriers";
 const char kTagCurrentCarrierIndex[] = "currentCarrierIndex";
 const char kTagShared[] = "shared";
 const char kTagShowActivateButton[] = "showActivateButton";
-const char kTagShowStaticIPConfig[] = "showStaticIPConfig";
 const char kTagShowViewAccountButton[] = "showViewAccountButton";
 const char kTagSimCardLockEnabled[] = "simCardLockEnabled";
 const char kTagSupportUrl[] = "supportUrl";
 const char kTagTrue[] = "true";
 const char kTagUsername[] = "username";
 const char kTagValue[] = "value";
 const char kTagVpnList[] = "vpnList";
 const char kTagWifiAvailable[] = "wifiAvailable";
 const char kTagWifiEnabled[] = "wifiEnabled";
 const char kTagWimaxAvailable[] = "wimaxAvailable";
@@ skipping 110 matching lines @@
   std::string name_servers;
   if (shill_properties.GetStringWithoutPathExpansion(
       name_servers_key, &name_servers)) {
     ip_info_dict->SetString(kIpConfigNameServers, name_servers);
     VLOG(2) << "Found " << name_servers_key << ": " << name_servers;
   }
 
   return ip_info_dict.release();
 }
 
-// Decorate dictionary |value_dict| with policy information from |ui_data|.
-void DecorateValueDictionary(const NetworkPropertyUIData& ui_data,
-                             const base::Value& value,
-                             base::DictionaryValue* value_dict) {
-  const base::Value* recommended_value = ui_data.default_value();
-  if (ui_data.IsManaged())
-    value_dict->SetString(kTagControlledBy, kTagPolicy);
-  else if (recommended_value && recommended_value->Equals(&value))
-    value_dict->SetString(kTagControlledBy, kTagRecommended);
-
-  if (recommended_value)
-    value_dict->Set(kTagRecommendedValue, recommended_value->DeepCopy());
-}
-
 // Decorate pref value as CoreOptionsHandler::CreateValueForPref() does and
 // store it under |key| in |settings|. Takes ownership of |value|.
 void SetValueDictionary(base::DictionaryValue* settings,
                         const char* key,
                         base::Value* value,
                         const NetworkPropertyUIData& ui_data) {
   base::DictionaryValue* dict = new base::DictionaryValue();
   // DictionaryValue::Set() takes ownership of |value|.
   dict->Set(kTagValue, value);
   settings->Set(key, dict);
-  DecorateValueDictionary(ui_data, *value, dict);
+
+  const base::Value* recommended_value = ui_data.default_value();
+  if (ui_data.IsManaged())
+    dict->SetString(kTagControlledBy, kTagPolicy);
+  else if (recommended_value && recommended_value->Equals(value))
+    dict->SetString(kTagControlledBy, kTagRecommended);
+
+  if (recommended_value)
+    dict->Set(kTagRecommendedValue, recommended_value->DeepCopy());
 }
 
-// Creates a decorated dictionary like SetValueDictionary does, but extended for
-// the Autoconnect property, which respects additionally global network policy.
-void SetAutoconnectValueDictionary(bool network_is_private,
-                                   ::onc::ONCSource onc_source,
-                                   bool current_autoconnect,
-                                   const NetworkPropertyUIData& ui_data,
-                                   base::DictionaryValue* settings) {
+// Creates a ManagedState style dictionary. Note(stevenjb): This is bridge code

[pneubeck (no reviews), 2014/07/30 20:12:46]

here and in the commit message:
ManagedState -> ManagedProperties ?

ManagedState sounds like chromeos/network/managed_state??

[stevenjb, 2014/07/30 21:57:48]

Done.

+// until we use GetManagedProperties to retrieve all Shill properties.
+void SetManagedValueDictionary(base::DictionaryValue* settings,

[pneubeck (no reviews), 2014/07/30 20:12:46]

nit: should be the last argument

[stevenjb, 2014/07/30 21:57:48]

Fixed here and for SetValueDictionary.

+                               const char* key,

[pneubeck (no reviews), 2014/07/30 20:12:46]

const std::string&

[stevenjb, 2014/07/30 21:57:48]

Why?

[pneubeck (no reviews), 2014/07/31 20:29:36]

This CL has more important changes, that I will spare you any elaboration.
Should have marked it as 'optional nit'

+                               base::Value* value,

[pneubeck (no reviews), 2014/07/30 20:12:46]

scoped_ptr (it's actually called with .release() )

[stevenjb, 2014/07/30 21:57:48]

I'm leaving this consistent with SetValueDictionary. I don't agree that adding
scoped_ptr overhead to this file local helper function is more readable.

[pneubeck (no reviews), 2014/07/31 20:29:36]

it's documenting ownership.
Otherwise this should probably have a comment.

[stevenjb, 2014/08/01 00:53:41]

Commented.

+                               const NetworkPropertyUIData& ui_data) {
   base::DictionaryValue* dict = new base::DictionaryValue();
-  base::Value* value = new base::FundamentalValue(current_autoconnect);
-  // DictionaryValue::Set() takes ownership of |value|.
-  dict->Set(kTagValue, value);
-  settings->Set(kTagAutoConnect, dict);
-  if (onc_source != ::onc::ONC_SOURCE_USER_POLICY &&
-      onc_source != ::onc::ONC_SOURCE_DEVICE_POLICY) {
-    // Autoconnect can be controlled by the GlobalNetworkConfiguration of the
-    // ONC policy.
-    bool only_policy_autoconnect =
-        onc::PolicyAllowsOnlyPolicyNetworksToAutoconnect(network_is_private);
-    if (only_policy_autoconnect) {
-      dict->SetString(kTagControlledBy, kTagPolicy);
-      return;
-    }
+  settings->Set(key, dict);
+
+  dict->Set(::onc::kAugmentationActiveSetting, value);  // owns |value|
+  if (ui_data.default_value()) {
+    std::string effective;
+    if (ui_data.onc_source() == ::onc::ONC_SOURCE_DEVICE_POLICY)

[pneubeck (no reviews), 2014/07/30 20:12:46]

I fear that this is not fully right.
Looking at NetworkPropertyUIData,
  - onc_source() is by default the source of the policy
  - if this particular property is 'recommended', onc_source() is NONE instead.
  - default_value() is only set if the property is 'recommended'
  - if the property is not recommended it was assumed that the policy is
enforced anyways, and the actual value == policy value

Thus, this function should probably look like:

void SetManagedValueDictionary(OncSource onc_source, bool is_private, .... ) {
  DCHECK(value);
  ...
  dict->Set(::onc::kAugmentationActiveSetting, value);  // Fine, as value comes
directly from Shill

  // We don't have a better source for the user's setting than the actual value
from Shill
  std::string effective = is_private ? UserSetting : SharedSetting;     
  dict->Set(effective, value->DeepCopy());

  if (onc_source == ...NONE) {
     // No policy to consider.
  } else {
    std::string policy_key = onc_source == USER_POLICY ?
              ::onc::kAugmentationUserPolicy : ...DevicePolicy;
    if (ui_data.onc_source() == ...NONE) {
       // The policy 'recommends' this property, thus the value is editable by
the user.
       dict->SetBoolean(onc_source == USER_POLICY ? kAugmentationUserEditable :
                     kAugmentationDeviceEditable, true);
       // ui_data.default_value contains the original policy value, even if
       // the user changed it. If the policy didn't set any value, then
default_value is NULL.
       if (ui_data.default_value())
         dict->Set(policy_key, ui_data.default_value()->DeepCopy());
       // If the recommended value equals the actual value from Shill, we don't
know 
       // whether the user has set this property explicitly or not. Assume that
the 
       // user didn't modify it.
       if (value->Equals(ui_data.default_value()))
         effective = policy_key;
    } else {
       // Don't set the default Editable value of false.
       // The actual value should equal the enforce policy value. Set it here
for completeness.
       dict->Set(policy_key, value->DeepCopy());
    }
  }
  dict->SetString(::onc::kAugmentationEffectiveSetting, effective);
}

[stevenjb, 2014/07/30 21:57:48]

So, this is an intermediate helper function, and eventually all
SetValueDictionary calls will be replaced with it (in places where we can't
immediately/easily use the results of GetManagedProperties directly). What is
the source of 'is_private' in these cases? I explicitly mangled the AutoConnect
code to modify ui_data t avoid passing that argument.

I don't see any reference to UserSetting or SharedSetting in the existing
SetValueDictionary or SetAutoconnectValueDictionary. I really don't want to add
functionality I don't fully understand here, I would much rather do that in a
followup. (Again, unless I am missing something?)

[pneubeck (no reviews), 2014/07/31 20:29:36]

In SetValueDictionary, UserSetting / SharedSetting can't be mentioned as it's
following the format of CoreOptionsHandler::CreateValueForPref .

Maybe you meant that you didn't see it in current calls to getManagedProperties
of the API.
You're right, that it doesn't appear there yet, because we still don't store
user settings separately from the Service properties in Shill.
It should then be sufficient to have combinations of these fields.
Without policy:
 Active, Effective (for now without UserSetting/SharedSetting)
With user (device) policy additionally:
 UserPolicy (DevicePolicy), UserEditable (DeviceEditable)

is_private: Whether the network is shared or not (stored in the user's profile
or the default profile)
onc_source: The source of the policy that controls the (whole) network, or NONE
if not present. This is different from ui_data.onc_source() as explained
somewhere else. The latter is NONE, if this particular property is recommended
by policy.

[stevenjb, 2014/08/01 00:53:41]

I think, now that I have written it all out in the doc I shared, that I
understand this better and am at least closer to something that is at least
partly correct.

+      effective = ::onc::kAugmentationDevicePolicy;
+    else if (ui_data.onc_source() == ::onc::ONC_SOURCE_USER_POLICY)
+      effective = ::onc::kAugmentationUserPolicy;
+    else
+      effective = ::onc::kAugmentationUnmanaged;

[pneubeck (no reviews), 2014/07/30 20:12:46]

could potentially also be kAugmentationUserSetting or ..SharedSettting .
On this side it might be ok to keep this, but on the .js side you will have to
handle either value at some time.

[stevenjb, 2014/07/30 21:57:48]

Acknowledged.

+    dict->SetString(::onc::kAugmentationEffectiveSetting, effective);
+    dict->Set(effective, ui_data.default_value()->DeepCopy());
   }
-  DecorateValueDictionary(ui_data, *value, dict);
 }
 
 std::string CopyStringFromDictionary(const base::DictionaryValue& source,
                                      const std::string& src_key,
                                      const std::string& dest_key,
                                      base::DictionaryValue* dest) {
   std::string string_value;
   if (source.GetStringWithoutPathExpansion(src_key, &string_value))
     dest->SetStringWithoutPathExpansion(dest_key, string_value);
   return string_value;
@@ skipping 64 matching lines @@
     // the Generic UMTS carrier option if present.
     if (gsm && (value == shill::kCarrierGenericUMTS))
       return index;
     // For other carriers, the service name will match the carrier name.
     if (value == device->carrier())
       return index;
   }
   return -1;
 }
 
-void PopulateWimaxDetails(const NetworkState* wimax,
-                          const base::DictionaryValue& shill_properties,
-                          base::DictionaryValue* dictionary) {
-  CopyStringFromDictionary(
-      shill_properties, shill::kEapIdentityProperty, kTagIdentity, dictionary);
-}
-
 void CreateDictionaryFromCellularApn(const base::DictionaryValue* apn,
                                      base::DictionaryValue* dictionary) {
   CopyStringFromDictionary(*apn, shill::kApnProperty, kTagApn, dictionary);
   CopyStringFromDictionary(
       *apn, shill::kApnNetworkIdProperty, kTagNetworkId, dictionary);
   CopyStringFromDictionary(
       *apn, shill::kApnUsernameProperty, kTagUsername, dictionary);
   CopyStringFromDictionary(
       *apn, shill::kApnPasswordProperty, kTagPassword, dictionary);
   CopyStringFromDictionary(*apn, shill::kApnNameProperty, kTagName, dictionary);
@@ skipping 164 matching lines @@
   // TODO(stevenjb): Once we eliminate all references to Shill properties,
   // we can switch to using managed_network_configuration_handler which
   // includes Device properties for Cellular (and skip the call to
   // onc::TranslateShillServiceToONCPart). For now we copy them over here.
   scoped_ptr<base::DictionaryValue> shill_properties_with_device(
       shill_properties.DeepCopy());
   const DeviceState* device =
       NetworkHandler::Get()->network_state_handler()->GetDeviceState(
           network->device_path());
   if (device) {
-    shill_properties_with_device->Set(shill::kDeviceProperty,
-                                      device->properties().DeepCopy());
+    shill_properties_with_device->SetWithoutPathExpansion(
+        shill::kDeviceProperty, device->properties().DeepCopy());
+    // Get the hardware MAC address from the DeviceState.
+    // (Note: this is done in ManagedNetworkConfigurationHandler but not
+    //  in NetworkConfigurationHandler).
+    if (!device->mac_address().empty()) {
+      shill_properties_with_device->SetStringWithoutPathExpansion(
+          shill::kAddressProperty, device->mac_address());
+    }
   }
   scoped_ptr<base::DictionaryValue> dictionary =
       onc::TranslateShillServiceToONCPart(
           *shill_properties_with_device, &onc::kNetworkWithStateSignature);
 
   dictionary->SetString(kNetworkInfoKeyServicePath, network->path());
   dictionary->SetString(
-      kTagErrorState,
+      kTagErrorMessage,
       ash::network_connect::ErrorString(network->error(), network->path()));
 
   dictionary->SetBoolean(kTagRemembered, !network->profile_path().empty());
   bool shared = !network->IsPrivate();
   dictionary->SetBoolean(kTagShared, shared);
 
   const std::string& type = network->type();
 
   const NetworkState* connected_network =
       NetworkHandler::Get()->network_state_handler()->ConnectedNetworkByType(
           NetworkTypePattern::Primitive(type));
   dictionary->SetBoolean(kTagDeviceConnected, connected_network != NULL);
 
-  if (type == shill::kTypeWimax)
-    PopulateWimaxDetails(network, shill_properties, dictionary.get());
-  else if (type == shill::kTypeCellular)
+  if (type == shill::kTypeCellular)
     PopulateCellularDetails(network, shill_properties, dictionary.get());
   else if (type == shill::kTypeVPN)
     PopulateVPNDetails(network, shill_properties, dictionary.get());
 
   return dictionary.Pass();
 }
 
 // Helper methods for SetIPConfigProperties
 bool AppendPropertyKeyIfPresent(const std::string& key,
                                 const base::DictionaryValue& old_properties,
@@ skipping 641 matching lines @@
     name_server_type = kNameServerTypeGoogle;
   }
   SetValueDictionary(dictionary.get(),
                      kDictionaryStaticIp,
                      static_ip_dict.release(),
                      property_ui_data);
 
   dictionary->SetString(kTagNameServerType, name_server_type);
   dictionary->SetString(kTagNameServersGoogle, kGoogleNameServers);
 
-  // Enable static ip config for Ethernet or WiFi.
-  bool staticIPConfig = network->Matches(NetworkTypePattern::Ethernet()) ||
-                        network->Matches(NetworkTypePattern::WiFi());
-  dictionary->SetBoolean(kTagShowStaticIPConfig, staticIPConfig);
-
   int priority = 0;
   shill_properties.GetIntegerWithoutPathExpansion(
       shill::kPriorityProperty, &priority);
   bool preferred = priority > 0;
   SetValueDictionary(dictionary.get(), kTagPreferred,
                      new base::FundamentalValue(preferred),
                      property_ui_data);
 
   NetworkPropertyUIData auto_connect_ui_data(onc_source);
   std::string onc_path_to_auto_connect;
@@ skipping 23 matching lines @@
           "Options_NetworkShowDetailsCellularConnected"));
     }
   }
   if (!onc_path_to_auto_connect.empty()) {
     auto_connect_ui_data.ParseOncProperty(
         onc_source, onc, onc_path_to_auto_connect);
   }
   bool auto_connect = false;
   shill_properties.GetBooleanWithoutPathExpansion(
       shill::kAutoConnectProperty, &auto_connect);
-  SetAutoconnectValueDictionary(network->IsPrivate(),
-                                onc_source,
-                                auto_connect,
-                                auto_connect_ui_data,
-                                dictionary.get());
+
+  // Autoconnect can be controlled by the GlobalNetworkConfiguration of the
+  // ONC policy.
+  scoped_ptr<base::Value> auto_connect_value(
+      new base::FundamentalValue(auto_connect));
+  if (!auto_connect_ui_data.IsManaged() &&

[pneubeck (no reviews), 2014/07/30 20:12:46]

don't use this IsManaged, as it's referring to the properties state only (which
could be recommended -> IsManaged == false).
Use onc_source != ..NONE instead.

[stevenjb, 2014/07/30 21:57:48]

Done.

[pneubeck (no reviews), 2014/07/31 20:29:36]

I meant the onc_source variable in this functions scope, not the member of
auto_connect_ui_data.

[stevenjb, 2014/08/01 00:53:41]

GGAHHHHHHH! I get it, I think, but.... I have to tell you that having a
NetworkPropertyUIData class that has an onc_source_ member that needs to be
accompanied by another onc_source argument seems... with all due respect.... bat
shit crazy.

[pneubeck (no reviews), 2014/08/01 16:09:07]

Maybe you guessed it already but this behavior of *UIData pre-dates my time at
this company. Feel free to run a 'git blame' ;-)

I fully agree that this is in a bad state. That's one of the reasons for
migrating to the ONC/getManagedProperties stuff.
Hopefully you don't find such crazyness there :-)

+      onc::PolicyAllowsOnlyPolicyNetworksToAutoconnect(network->IsPrivate())) {
+    ::onc::ONCSource source = network->IsPrivate()

[pneubeck (no reviews), 2014/07/30 20:12:46]

Please add a comment (which I should have done in the old code as well):

// This modifies auto_connect_ui_data, so that the autoconnect value appears as
set to 'false' by policy and not editable by the user.

[stevenjb, 2014/07/30 21:57:48]

Done.

+        ? ::onc::ONC_SOURCE_USER_POLICY : ::onc::ONC_SOURCE_DEVICE_POLICY;
+    auto_connect_ui_data.SetOncSourceAndValue(source, auto_connect_value.get());

[pneubeck (no reviews), 2014/07/30 20:12:46]

set the value to 'false' (although it should be the same if there was no bug in
the policy application).
If you want to, add a DCHECK to ensure this.

[stevenjb, 2014/07/30 21:57:48]

Done.

+  }
+  SetManagedValueDictionary(dictionary.get(),
+                            shill::kAutoConnectProperty,
+                            auto_connect_value.release(),
+                            auto_connect_ui_data);
 
   // Show details dialog
   web_ui()->CallJavascriptFunction(kShowDetailedInfoFunction, *dictionary);
 }
 
 gfx::NativeWindow InternetOptionsHandler::GetNativeWindow() const {
   return web_ui()->GetWebContents()->GetTopLevelNativeWindow();
 }
 
 float InternetOptionsHandler::GetScaleFactor() const {
@@ skipping 180 matching lines @@
   dictionary->SetBoolean(
       kTagWimaxAvailable,
       handler->IsTechnologyAvailable(NetworkTypePattern::Wimax()));
   dictionary->SetBoolean(
       kTagWimaxEnabled,
       handler->IsTechnologyEnabled(NetworkTypePattern::Wimax()));
 }
 
 }  // namespace options
 }  // namespace chromeos