Check VPN certificate before checking username/password

Also removes a redundant check for shill::kHostProperty

BUG=299643

Review URL: https://codereview.chromium.org/25101002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226190 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 20 insertions, 22 deletions

diff --git a/chromeos/network/network_connection_handler.cc b/chromeos/network/network_connection_handler.cc
index e6889f0..263a000 100644
--- a/chromeos/network/network_connection_handler.cc
+++ b/chromeos/network/network_connection_handler.cc
@@ -47,30 +47,23 @@ bool IsAuthenticationError(const std::string& error) {
           error == shill::kErrorEapAuthenticationFailed);
 }
 
-bool VPNIsConfigured(const std::string& service_path,
-                     const std::string& provider_type,
-                     const base::DictionaryValue& provider_properties) {
+bool VPNRequiresCredentials(const std::string& service_path,
+                           const std::string& provider_type,
+                           const base::DictionaryValue& provider_properties) {
   if (provider_type == shill::kProviderOpenVpn) {
-    std::string hostname;
-    provider_properties.GetStringWithoutPathExpansion(
-        shill::kHostProperty, &hostname);
-    if (hostname.empty()) {
-      NET_LOG_EVENT("OpenVPN: No hostname", service_path);
-      return false;
-    }
     std::string username;
     provider_properties.GetStringWithoutPathExpansion(
         shill::kOpenVPNUserProperty, &username);
     if (username.empty()) {
       NET_LOG_EVENT("OpenVPN: No username", service_path);
-      return false;
+      return true;
     }
     bool passphrase_required = false;
     provider_properties.GetBooleanWithoutPathExpansion(
         shill::kPassphraseRequiredProperty, &passphrase_required);
     if (passphrase_required) {
       NET_LOG_EVENT("OpenVPN: Passphrase Required", service_path);
-      return false;
+      return true;
     }
     NET_LOG_EVENT("OpenVPN Is Configured", service_path);
   } else {
@@ -80,11 +73,11 @@ bool VPNIsConfigured(const std::string& service_path,
         shill::kL2tpIpsecPskRequiredProperty, &passphrase_required);
     if (passphrase_required) {
       NET_LOG_EVENT("VPN: PSK Required", service_path);
-      return false;
+      return true;
     }
     NET_LOG_EVENT("VPN Is Configured", service_path);
   }
-  return true;
+  return false;
 }
 
 }  // namespace
@@ -347,11 +340,11 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect(
 
   // Get VPN provider type and host (required for configuration) and ensure
   // that required VPN non-cert properties are set.
+  const base::DictionaryValue* provider_properties = NULL;
   std::string vpn_provider_type, vpn_provider_host;
   if (type == shill::kTypeVPN) {
     // VPN Provider values are read from the "Provider" dictionary, not the
     // "Provider.Type", etc keys (which are used only to set the values).
-    const base::DictionaryValue* provider_properties;
     if (service_properties.GetDictionaryWithoutPathExpansion(
             shill::kProviderProperty, &provider_properties)) {
       provider_properties->GetStringWithoutPathExpansion(
@@ -363,13 +356,6 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect(
       ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
       return;
     }
-    // VPN requires a host and username to be set.
-    if (!VPNIsConfigured(
-            service_path, vpn_provider_type, *provider_properties)) {
-      NET_LOG_ERROR("VPN Not Configured", service_path);
-      ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
-      return;
-    }
   }
 
   client_cert::ConfigType client_cert_type = client_cert::CONFIG_TYPE_NONE;
@@ -441,6 +427,18 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect(
     }
   }
 
+  if (type == shill::kTypeVPN) {
+    // VPN may require a username, and/or passphrase to be set. (Check after
+    // ensuring that any required certificates are configured).
+    DCHECK(provider_properties);
+    if (VPNRequiresCredentials(
+            service_path, vpn_provider_type, *provider_properties)) {
+      NET_LOG_USER("VPN Requires Credentials", service_path);
+      ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
+      return;
+    }
+  }
+
   if (!config_properties.empty()) {
     NET_LOG_EVENT("Configuring Network", service_path);
     network_configuration_handler_->SetProperties(