Set onc_source UI data parameter when importing ONC.

chrome/browser/chromeos/cros/onc_network_parser.cc

 // Copyright (c) 2011 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/chromeos/cros/onc_network_parser.h"
 
 #include "base/base64.h"
 #include "base/json/json_value_serializer.h"
-#include "base/json/json_writer.h"  // for debug output only.
+#include "base/json/json_writer.h"
 #include "base/stringprintf.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/cros/native_network_constants.h"
 #include "chrome/browser/chromeos/cros/native_network_parser.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "net/base/cert_database.h"
 #include "net/base/crypto_module.h"
 #include "net/base/net_errors.h"
 #include "net/base/x509_certificate.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
@@ skipping 16 matching lines @@
 
 OncValueSignature network_configuration_signature[] = {
   // TODO(crosbug.com/23673): Support Ethernet settings.
   { "GUID", PROPERTY_INDEX_GUID, TYPE_STRING },
   { "Name", PROPERTY_INDEX_NAME, TYPE_STRING },
   // TODO(crosbug.com/23674): Support ProxySettings.
   // TODO(crosbug.com/23604): Handle removing networks.
   { "Remove", PROPERTY_INDEX_ONC_REMOVE, TYPE_BOOLEAN },
   { "Type", PROPERTY_INDEX_TYPE, TYPE_STRING },
   { "WiFi", PROPERTY_INDEX_ONC_WIFI, TYPE_DICTIONARY },
-  { "VPN", PROPERTY_INDEX_ONC_VPN, TYPE_DICTIONARY }
+  { "VPN", PROPERTY_INDEX_ONC_VPN, TYPE_DICTIONARY },
+  { NULL }

[kmixter1, 2011/12/06 12:52:37]

Ouch, nice catch.

 };
 
 OncValueSignature wifi_signature[] = {
-  { "AutoConnect", PROPERTY_INDEX_AUTO_CONNECT, TYPE_BOOLEAN },
+  { "AutoConnect", PROPERTY_INDEX_AUTO_CONNECT, TYPE_BOOLEAN,
+    NetworkUIData::kPropertyAutoConnect },
   { "EAP", PROPERTY_INDEX_EAP, TYPE_DICTIONARY },
   { "HiddenSSID", PROPERTY_INDEX_HIDDEN_SSID, TYPE_BOOLEAN },
-  { "Passphrase", PROPERTY_INDEX_PASSPHRASE, TYPE_STRING },
+  { "Passphrase", PROPERTY_INDEX_PASSPHRASE, TYPE_STRING,
+    NetworkUIData::kPropertyPassphrase },
   { "ProxyURL", PROPERTY_INDEX_PROXY_CONFIG, TYPE_STRING },
   { "Security", PROPERTY_INDEX_SECURITY, TYPE_STRING },
   { "SSID", PROPERTY_INDEX_SSID, TYPE_STRING },
   { NULL }
 };
 
 OncValueSignature eap_signature[] = {
-  { "AnonymousIdentity", PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY, TYPE_STRING },
+  { "AnonymousIdentity", PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY, TYPE_STRING,
+    NetworkUIData::kPropertyEAPAnonymousIdentity },

[kmixter1, 2011/12/06 12:52:37]

why can't these use the same name as in ONC?

[Mattias Nissler (ping if slow), 2011/12/06 13:37:59]

See other comment.

   { "ClientCertPattern", PROPERTY_INDEX_ONC_CLIENT_CERT_PATTERN,
     TYPE_DICTIONARY },
   { "ClientCertRef", PROPERTY_INDEX_ONC_CLIENT_CERT_REF, TYPE_STRING },
   { "ClientCertType", PROPERTY_INDEX_ONC_CLIENT_CERT_TYPE, TYPE_STRING },
-  { "Identity", PROPERTY_INDEX_EAP_IDENTITY, TYPE_STRING },
-  { "Inner", PROPERTY_INDEX_EAP_PHASE_2_AUTH, TYPE_STRING },
-  { "Outer", PROPERTY_INDEX_EAP_METHOD, TYPE_STRING },
-  { "Password", PROPERTY_INDEX_EAP_PASSWORD, TYPE_STRING },
-  { "ServerCARef", PROPERTY_INDEX_EAP_CA_CERT, TYPE_STRING },
-  { "UseSystemCAs", PROPERTY_INDEX_EAP_USE_SYSTEM_CAS, TYPE_BOOLEAN },
+  { "Identity", PROPERTY_INDEX_EAP_IDENTITY, TYPE_STRING,
+    NetworkUIData::kPropertyEAPIdentity },
+  { "Inner", PROPERTY_INDEX_EAP_PHASE_2_AUTH, TYPE_STRING,
+    NetworkUIData::kPropertyEAPPhase2Auth },
+  { "Outer", PROPERTY_INDEX_EAP_METHOD, TYPE_STRING,
+    NetworkUIData::kPropertyEAPMethod },
+  { "Password", PROPERTY_INDEX_EAP_PASSWORD, TYPE_STRING,
+    NetworkUIData::kPropertyEAPPassphrase },
+  { "ServerCARef", PROPERTY_INDEX_EAP_CA_CERT, TYPE_STRING,
+    NetworkUIData::kPropertyEAPServerCaCertNssNickname },
+  { "UseSystemCAs", PROPERTY_INDEX_EAP_USE_SYSTEM_CAS, TYPE_BOOLEAN,
+    NetworkUIData::kPropertyEAPUseSystemCAs },
   { NULL }
 };
 
 OncValueSignature vpn_signature[] = {
   { "Host", PROPERTY_INDEX_PROVIDER_HOST, TYPE_STRING },
   { "IPsec", PROPERTY_INDEX_ONC_IPSEC, TYPE_DICTIONARY },
   { "L2TP", PROPERTY_INDEX_ONC_L2TP, TYPE_DICTIONARY },
   { "OpenVPN", PROPERTY_INDEX_ONC_OPENVPN, TYPE_DICTIONARY },
   { "Type", PROPERTY_INDEX_PROVIDER_TYPE, TYPE_STRING },
   { NULL }
@@ skipping 72 matching lines @@
   };
   CR_DEFINE_STATIC_LOCAL(EnumMapper<ConnectionType>, parser,
       (table, arraysize(table), TYPE_UNKNOWN));
   return parser.Get(type);
 }
 
 }  // namespace
 
 // -------------------- OncNetworkParser --------------------
 
-OncNetworkParser::OncNetworkParser(const std::string& onc_blob)
+OncNetworkParser::OncNetworkParser(const std::string& onc_blob,
+                                   NetworkUIData::ONCSource onc_source)
     : NetworkParser(get_onc_mapper()),
+      onc_source_(onc_source),
       network_configs_(NULL),
       certificates_(NULL) {
   VLOG(2) << __func__ << ": OncNetworkParser called on " << onc_blob;
   JSONStringValueSerializer deserializer(onc_blob);
   deserializer.set_allow_trailing_comma(true);
   scoped_ptr<base::Value> root(deserializer.Deserialize(NULL, &parse_error_));
 
   if (!root.get() || root->GetType() != base::Value::TYPE_DICTIONARY) {
     LOG(WARNING) << "OncNetworkParser received bad ONC file: " << parse_error_;
   } else {
@@ skipping 26 matching lines @@
 }
 
 int OncNetworkParser::GetNetworkConfigsSize() const {
   return network_configs_ ? network_configs_->GetSize() : 0;
 }
 
 int OncNetworkParser::GetCertificatesSize() const {
   return certificates_ ? certificates_->GetSize() : 0;
 }
 
+Network* OncNetworkParser::ParseNetwork(int n) {
+  if (!network_configs_)
+    return NULL;
+  DictionaryValue* info = NULL;
+  if (!network_configs_->GetDictionary(n, &info))
+    return NULL;
+  if (VLOG_IS_ON(2)) {
+    std::string network_json;
+    base::JSONWriter::Write(static_cast<base::Value*>(info),
+                            true, &network_json);
+    VLOG(2) << "Parsing network at index " << n
+            << ": " << network_json;
+  }
+
+  return CreateNetworkFromInfo(std::string(), *info);
+}
+
 bool OncNetworkParser::ParseCertificate(int cert_index) {
   CHECK(certificates_);
   CHECK(static_cast<size_t>(cert_index) < certificates_->GetSize());
   CHECK(cert_index >= 0);
   base::DictionaryValue* certificate = NULL;
   certificates_->GetDictionary(cert_index, &certificate);
   CHECK(certificate);
 
   if (VLOG_IS_ON(2)) {
     std::string certificate_json;
@@ skipping 27 matching lines @@
   }
   if (cert_type == "Client") {
     return ParseClientCertificate(cert_index, certificate);
   }
 
   LOG(WARNING) << "ONC File: certificate of unknown type: " << cert_type
                << " at index " << cert_index;
   return false;
 }
 
-Network* OncNetworkParser::ParseNetwork(int n) {
-  if (!network_configs_)
-    return NULL;
-  DictionaryValue* info = NULL;
-  if (!network_configs_->GetDictionary(n, &info))
-    return NULL;
-  if (VLOG_IS_ON(2)) {
-    std::string network_json;
-    base::JSONWriter::Write(static_cast<base::Value*>(info),
-                            true, &network_json);
-    VLOG(2) << "Parsing network at index " << n
-            << ": " << network_json;
-  }
-
-  return CreateNetworkFromInfo(std::string(), *info);
-}
-
 Network* OncNetworkParser::CreateNetworkFromInfo(
     const std::string& service_path,
     const DictionaryValue& info) {
   ConnectionType type = ParseTypeFromDictionary(info);
   if (type == TYPE_UNKNOWN)  // Return NULL if cannot parse network type.
     return NULL;
   scoped_ptr<Network> network(CreateNewNetwork(type, service_path));
+
+  // Initialize UI data.
+  NetworkUIData ui_data;
+  ui_data.set_onc_source(onc_source_);
+  ui_data.FillDictionary(network->ui_data());
+
+  // Parse all properties recursively.
   if (!ParseNestedObject(network.get(),
                          "NetworkConfiguration",
                          static_cast<const base::Value&>(info),
                          network_configuration_signature,
                          ParseNetworkConfigurationValue)) {
     LOG(WARNING) << "Network " << network->name() << " had problems.";
     return NULL;
   }
+
+  // Update the UI data property.
+  std::string ui_data_json;
+  base::JSONWriter::Write(network->ui_data(), false, &ui_data_json);
+  base::StringValue ui_data_string_value(ui_data_json);
+  network->UpdatePropertyMap(PROPERTY_INDEX_UI_DATA, ui_data_string_value);
+
   if (VLOG_IS_ON(2)) {
     VLOG(2) << "Created Network '" << network->name()
             << "' from info. Path:" << service_path
             << " Type:" << ConnectionTypeToString(type);
   }
+
   return network.release();
 }
 
 Network* OncNetworkParser::CreateNewNetwork(
     ConnectionType type, const std::string& service_path) {
   Network* network = NetworkParser::CreateNewNetwork(type, service_path);
   if (network) {
     if (type == TYPE_WIFI)
       network->SetNetworkParser(new OncWifiNetworkParser());
     else if (type == TYPE_VPN)
@@ skipping 131 matching lines @@
                  << " (error " << net::ErrorToString(result) << ").";
     return false;
   }
   VLOG(2) << "Successfully imported client certificate at index "
           << cert_index;
   return true;
 }
 
 bool OncNetworkParser::ParseNestedObject(Network* network,
                                          const std::string& onc_type,
                                          const base::Value& value,

[stevenjb, 2011/12/08 02:26:35]

|value| here is an ONC dictionary entry, yes? We should maybe name it better.
Since this has to be a dictionary, shouldn't we pass it as such? Also, this
appears to be a public function, but it seems like it should be private? 

                                          OncValueSignature* signature,
                                          ParserPointer parser) {
   bool any_errors = false;
   if (!value.IsType(base::Value::TYPE_DICTIONARY)) {
     VLOG(1) << network->name() << ": expected object of type " << onc_type;
     return false;
   }
   VLOG(2) << "Parsing nested object of type " << onc_type;
   const DictionaryValue* dict = NULL;
   value.GetAsDictionary(&dict);
@@ skipping 22 matching lines @@
     }
     PropertyIndex index = signature[field_index].index;
     // We need to UpdatePropertyMap now since parser might want to
     // change the mapped value.
     network->UpdatePropertyMap(index, *inner_value);
     if (!parser(this, index, *inner_value, network)) {
       VLOG(1) << network->name() << ": problem(s) with field: " << key;
       any_errors = true;
       continue;
     }
+
     if (VLOG_IS_ON(2)) {
       std::string value_json;
       base::JSONWriter::Write(inner_value, true, &value_json);
       VLOG(2) << network->name() << ": Successfully parsed [" << key
               << "(" << index << ")] = " << value_json;
     }
   }
+
+  ParseUIData(network, dict, signature);
+
   return !any_errors;
 }
 
 // static
 bool OncNetworkParser::CheckNetworkType(Network* network,
                                         ConnectionType expected,
                                         const std::string& onc_type) {
   if (expected != network->type()) {
     VLOG(2) << network->name()
             << ": " << onc_type << " field unexpected for this type network";
@@ skipping 76 matching lines @@
     case PROPERTY_INDEX_GUID:
     case PROPERTY_INDEX_NAME:
       // Fall back to generic parser for these.
       return parser->ParseValue(index, value, network);
     default:
       break;
   }
   return false;
 }
 
+void OncNetworkParser::ParseUIData(Network* network,
+                                   const base::DictionaryValue* dict,
+                                   OncValueSignature* signature) const {

[stevenjb, 2011/12/08 02:26:35]

|signature| is a list of signatures, yes, not a pointer to a single signature?
We should name it appropriately, e.g. signatures, or signature_list.

+  // Collect the set of recommended properties.
+  std::set<std::string> recommended;
+  bool all_recommended = false;
+  base::ListValue* recommended_list = NULL;
+  if (dict->GetList("Recommended", &recommended_list)) {
+    for (base::ListValue::const_iterator entry(recommended_list->begin());
+         entry != recommended_list->end(); ++entry) {
+      std::string entry_string;
+      if ((*entry)->GetAsString(&entry_string)) {
+        if (entry_string == ".") {
+          all_recommended = true;
+          break;
+        } else {
+          recommended.insert(entry_string);
+        }
+      } else {
+        LOG(WARNING) << "Entries in the recommended list should be strings!";
+      }
+    }
+  }
+
+  // Do a pass over |signature|, filling in UI data where applicable.
+  NetworkUIData ui_data;
+  for (int i = 0; signature[i].field != NULL; ++i) {
+    if (!signature[i].ui_data_key)
+      continue;
+
+    bool controlled_by_policy =
+        (onc_source_ == NetworkUIData::ONC_SOURCE_DEVICE_POLICY ||
+         onc_source_ == NetworkUIData::ONC_SOURCE_USER_POLICY) &&
+        !all_recommended &&

[kmixter1, 2011/12/06 12:52:37]

I was thinking "." meant something a little different - not so much that every
field was recommended (as a shorthand) but that the entity itself was
recommended.  So a recommended certificate can be removed and a recommended
network can be forgotten.  But properties within either can still be mandatory. 
Does that make sense?

[Mattias Nissler (ping if slow), 2011/12/06 13:37:59]

Ah, so I didn't understand what your intention with "." was. Does having
recommended certs and networks actually make sense? For networks, I guess it
doesn't, since the user can still select which network to use anyway. Then, the
only reason for recommended networks would be for the user to be able to remove
network entries, but do we really want to support this use case? Also how, to
bring recommended networks back once you've removed them? Regarding certs, I
cannot see a use case for recommended certs at all. Is there one?

[kmixter1, 2011/12/07 23:08:29]

I added "." just for completeness in the specification.  With the way we were
specifying the Recommended field, we could express a value of any scalar field
as being recommended but we had no way to make things in arrays be recommended. 
Conveniently most things in arrays that could be made recommended were objects,
and so adding "." closed that gap.  Whether or not anyone would ever want it, I
don't know.  I'm quite happy to punt on that capability indefinitely.

+        recommended.find(signature[i].ui_data_key) == recommended.end();

[stevenjb, 2011/12/08 02:26:35]

I'm confused by this. Above, we appear to be filling in recommended<> from
dict["Recommended"], which is ONC data. Why are we searching for ui_data_key
here?

+    const Value* default_value = NULL;
+    network->GetProperty(signature[i].index, &default_value);
+    NetworkPropertyUIData property_ui_data(
+        controlled_by_policy ? NetworkPropertyUIData::CONTROLLER_POLICY :
+                               NetworkPropertyUIData::CONTROLLER_USER,
+        default_value ? default_value->DeepCopy() : NULL);
+    ui_data.SetProperty(signature[i].ui_data_key, property_ui_data);

[stevenjb, 2011/12/08 02:26:35]

I can see why it is convenient to save this off, and if we're not going to parse
ONC data on every login, then it makes sense to store it in flimflam, but I
don't understand why we aren't just using signature[i].field.

+  }
+
+  // Merge the result into the network-level |ui_data| dictionary.
+  DictionaryValue ui_data_dict;
+  ui_data.FillDictionary(&ui_data_dict);
+  network->ui_data()->MergeDictionary(&ui_data_dict);
+}
+
 // -------------------- OncWirelessNetworkParser --------------------
 
 OncWirelessNetworkParser::OncWirelessNetworkParser() {}
 OncWirelessNetworkParser::~OncWirelessNetworkParser() {}
 
 // -------------------- OncWifiNetworkParser --------------------
 
 OncWifiNetworkParser::OncWifiNetworkParser() {}
 
 OncWifiNetworkParser::~OncWifiNetworkParser() {}
@@ skipping 433 matching lines @@
     // on the value of AuthenticationType.
     { "L2TP-IPsec", PROVIDER_TYPE_L2TP_IPSEC_PSK },
     { "OpenVPN", PROVIDER_TYPE_OPEN_VPN },
   };
   CR_DEFINE_STATIC_LOCAL(EnumMapper<ProviderType>, parser,
       (table, arraysize(table), PROVIDER_TYPE_MAX));
   return parser.Get(type);
 }
 
 }  // namespace chromeos