MD Settings: Add settings-internet-config (WiFi only, no certs)

chrome/browser/resources/settings/internet_page/internet_config.js

+// Copyright 2015 The Chromium Authors. All rights reserved.

[michaelpg, 2017/05/18 21:14:23]

2017

[stevenjb, 2017/05/19 23:35:34]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/**
+ * @fileoverview
+ * 'settings-internet-config' provides configuration of authentication
+ * properties for new and existing networks.
+ */
+Polymer({
+  is: 'settings-internet-config',
+
+  behaviors: [settings.RouteObserverBehavior, I18nBehavior],
+
+  properties: {
+    /**
+     * Interface for networkingPrivate calls, passed from internet_page.
+     * @type {NetworkingPrivate}
+     */
+    networkingPrivate: Object,
+
+    /**
+     * The GUID when an existing network is being configured. This will be
+     * empty when configuring a new network.
+     * @private
+     */
+    guid_: String,
+
+    /**
+     * The current properties if an existing network being configured.
+     * This will be undefined when configuring a new network.
+     * @type {!chrome.networkingPrivate.NetworkProperties|undefined}
+     * @private
+     */
+    networkProperties_: Object,
+
+    /**
+     * The configuraiton properties for the network. |configProperties_.Type|
+     * will always be defined as the network type being configured.

[michaelpg, 2017/05/18 21:14:23]

"always": including when configuring a new network?

if so, remove |undefined in @type

[stevenjb, 2017/05/19 23:35:34]

Yes. Done. (It gets set in currentRouteChanged, so technically can be briefly
undefined, but closure doesn't seem to care, so... works for me).

+     * @type {!chrome.networkingPrivate.NetworkConfigProperties|undefined}
+     * @private
+     */
+    configProperties_: Object,
+
+    /**
+     * The title to display (network name or type).
+     * @private
+     */
+    title_: String,
+
+    /**
+     * Whether this network should be shared with other users of the device.
+     * @private
+     */
+    shareNetwork_: Boolean,
+
+    /**
+     * Saved security value, used to detect when Security changes.
+     * @private
+     */
+    savedSecurity_: String,
+
+    /**
+     * Object providing network type values for data binding.
+     * @const
+     * @private
+     */
+    NetworkType_: {

[michaelpg, 2017/05/18 21:14:23]

This isn't really used, apart from passing WI_FI to isType_() which could more
simply be "isWiFi_()". Consider removing/simplifying for now.

[stevenjb, 2017/05/19 23:35:34]

We will definitely have other values in the very soon. Removed CELLULAR which
won't be used and added a comment.

+      type: Object,
+      value: {
+        CELLULAR: CrOnc.Type.CELLULAR,
+        ETHERNET: CrOnc.Type.ETHERNET,
+        VPN: CrOnc.Type.VPN,
+        WI_FI: CrOnc.Type.WI_FI,
+        WI_MAX: CrOnc.Type.WI_MAX,
+      },
+      readOnly: true
+    },
+
+    /**
+     * Array of values for the WiFi Security dropdown.
+     * @type {!Array<string>}
+     * @const
+     * @private
+     */
+    securityItems_: {
+      type: Array,
+      readOnly: true,
+      value: function() {

[michaelpg, 2017/05/18 21:14:23]

if @const, doesn't really need to be returned via a function()

same for the below props

[stevenjb, 2017/05/19 23:35:34]

Done.

+        return [
+          CrOnc.Security.NONE, CrOnc.Security.WEP_PSK, CrOnc.Security.WPA_PSK,
+          CrOnc.Security.WPA_EAP
+        ];
+      }
+    },
+
+    /**
+     * Array of values for the EAP Method (Outer) dropdown.
+     * @type {!Array<string>}
+     * @const
+     * @private
+     */
+    eapOuterItems_: {
+      type: Array,
+      readOnly: true,
+      value: function() {
+        return ['LEAP', 'PEAP', 'EAP-TLS', 'EAP-TTLS'];
+      }
+    },
+
+    /**
+     * Array of values for the EAP EAP Phase 2 authentication (Inner) dropdown
+     * when the Outer type is PEAP.
+     * @type {!Array<string>}
+     * @const
+     * @private
+     */
+    eapInnerItemsPeap_: {
+      type: Array,
+      readOnly: true,
+      value: function() {
+        return ['Automatic', 'MD5', 'MSCHAPv2'];
+      }
+    },
+
+    /**
+     * Array of values for the EAP EAP Phase 2 authentication (Inner) dropdown
+     * when the Outer type is EAP-TTLS.
+     * @type {!Array<string>}
+     * @const
+     * @private
+     */
+    eapInnerItemsTtls_: {
+      type: Array,
+      readOnly: true,
+      value: function() {
+        return ['Automatic', 'MD5', 'MSCHAP', 'MSCHAPv2', 'PAP', 'CHAP', 'GTC'];

[michaelpg, 2017/05/18 21:14:23]

Suggestion: these constants may be more readable as a simple JS constant at the
top of the file: 
  var kEapInnerItemsTtls = ['...', ...];
  // etc
rather than fully fledged Polymer properties.

The file would certainly be smaller.

[stevenjb, 2017/05/19 23:35:34]

Hmm, the others are used directly in the html so need to be properties. The
inner ones do not, but I think I prefer the consistency. They are smaller now
that I am just declaring them inline instead of using a function.

+      }
+    },
+  },
+
+  observers: ['updateConfigProperties_(configProperties_.*)'],
+
+  /** @const */
+  MIN_PASSPHRASE_LENGTH: 5,
+
+  /**
+   * settings.RouteObserverBehavior
+   * @param {!settings.Route} route
+   * @protected
+   */
+  currentRouteChanged: function(route) {
+    if (route != settings.Route.NETWORK_CONFIG)
+      return;
+
+    var queryParams = settings.getQueryParameters();
+    this.guid_ = queryParams.get('guid') || '';
+
+    // Set networkProperties for new configurations and for existing
+    // configuraitons until the current properties are loaded.
+    var name = queryParams.get('name') || '';
+    var type = /** @type {!chrome.networkingPrivate.NetworkType} */ (
+                   queryParams.get('type')) ||
+        CrOnc.Type.WI_FI;
+    this.networkProperties_ = {
+      GUID: this.guid_,
+      Name: name,
+      Type: type,
+    };
+    var configProperties = {Type: type};
+    if (type == CrOnc.Type.WI_FI) {
+      configProperties.WiFi = {
+        SSID: '',
+        Security: CrOnc.Security.NONE,
+      };
+    }
+    this.configProperties_ = configProperties;
+    if (this.guid_) {
+      this.networkingPrivate.getProperties(
+          this.guid_, this.getPropertiesCallback_.bind(this));
+    } else {
+      this.title_ = this.i18n('OncType' + type);
+      this.shareNetwork_ = true;
+    }
+  },
+
+  /** @private */
+  close_: function() {
+    // Delay navigating to allow other subpages to load first (e.g. when called

[michaelpg, 2017/05/18 21:14:23]

I don't understand why this is necessary -- what happens if we navigate
immediately?

[stevenjb, 2017/05/19 23:35:34]

It's kind of an edge case, but if a page is left open to a no-longer visible
network we close it immediately which can look odd while the rest of Settings is
getting loaded. It's much more of an issue for the details page where this was
copied from, I'll go ahead and remove it here.

+    // from a getPropertiesCallback_ error).
+    requestAnimationFrame(function() {
+      settings.navigateToPreviousRoute();

[michaelpg, 2017/05/18 21:14:23]

verify that we are still on NETWORK_CONFIG so we don't randomly go "back" from
an unrelated page in response to getPropertiesCallback or something

[stevenjb, 2017/05/19 23:35:34]

Done.

+    });
+  },
+
+  /**
+   * networkingPrivate.getProperties callback.
+   * @param {!chrome.networkingPrivate.NetworkProperties} properties
+   * @private
+   */
+  getPropertiesCallback_: function(properties) {
+    if (!properties) {
+      // If |properties| is null, the network no longer exists; close the page.
+      console.error('Network no longer exists: ' + this.guid_);
+      this.close_();
+      return;
+    }
+    this.networkProperties_ = properties;
+    this.title_ = properties.Name || this.i18n('OncType' + properties.Type);
+    this.shareNetwork_ = properties.Source == CrOnc.Source.DEVICE ||
+        properties.Source == CrOnc.Source.DEVICE_POLICY;
+    this.savedSecurity_ = /** @type {string} */ (
+        this.get('WiFi.Security', this.networkProperties_));
+
+    var configProperties =
+        /** @type {chrome.networkingPrivate.NetworkConfigProperties} */ ({
+          Name: properties.Name,
+          Type: properties.Type,
+        });
+    if (properties.WiFi) {
+      configProperties.WiFi = {
+        AutoConnect: properties.WiFi.AutoConnect,
+        EAP: properties.WiFi.EAP,
+        Passphrase: properties.WiFi.Passphrase,
+        SSID: properties.WiFi.SSID,
+        Security: properties.WiFi.Security
+      };
+    }
+    this.configProperties_ = configProperties;
+  },
+
+  /** @private */
+  updateConfigProperties_: function() {
+    if (this.configProperties_.WiFi) {
+      var security = /** @type {string} */ (
+          this.get('WiFi.Security', this.configProperties_));
+      var eapOuter;
+      if (security == CrOnc.Security.WPA_EAP) {
+        this.configProperties_.WiFi.EAP = this.configProperties_.WiFi.EAP || {};
+        eapOuter = this.configProperties_.WiFi.EAP.Outer;
+        if (!eapOuter) {
+          eapOuter = 'LEAP';
+          this.configProperties_.WiFi.EAP.Outer = eapOuter;
+        }
+        if (eapOuter == 'PEAP' || eapOuter == 'EAP-TTLS') {

[michaelpg, 2017/05/18 21:14:23]

I think these strings are used frequently enough that they it'd be safer to make
them constants: kPeap, kEapTtls, etc.

[stevenjb, 2017/05/19 23:35:34]

Added to OncTypes.

+          if (!this.configProperties_.WiFi.EAP.Inner)
+            this.configProperties_.WiFi.EAP.Inner = 'Automatic';

[michaelpg, 2017/05/18 21:14:23]

please consider splitting this function up -- it has too many branches to easily
follow

[stevenjb, 2017/05/19 23:35:34]

Done.

+        } else {
+          delete this.configProperties_.WiFi.EAP.Inner;
+        }
+      } else {
+        delete this.configProperties_.WiFi.EAP;
+      }
+
+      if (security != this.savedSecurity_) {
+        if (!security || security == CrOnc.Security.NONE) {
+          this.shareNetwork_ = true;
+        } else {
+          // TODO: also check login state.

[michaelpg, 2017/05/18 21:14:23]

(stevenjb)

[stevenjb, 2017/05/19 23:35:34]

Done.

+          this.shareNetwork_ = false;
+        }
+        this.savedSecurity_ = security;
+      }
+    }
+    this.setShowEap_();
+  },
+
+  /**
+   * @param {CrOnc.Type} type The type to compare against.
+   * @param {CrOnc.Type} networkType The current network type.
+   * @return {boolean} True if the network type matches 'type'.
+   * @private
+   */
+  isType_: function(type, networkType) {
+    return type == networkType;
+  },
+
+  /**
+   * @return {boolean}
+   * @private
+   */
+  connectIsEnabled_: function() {
+    if (this.configProperties_.Type == CrOnc.Type.WI_FI) {
+      if (!this.get('WiFi.SSID', this.configProperties_))
+        return false;
+      if (this.showWiFiPassphrase_()) {
+        var passphrase = this.get('WiFi.Passphrase', this.configProperties_);
+        if (!passphrase || passphrase.length < this.MIN_PASSPHRASE_LENGTH)
+          return false;
+      }
+    }
+    // TODO(stevenjb): Check certificates.
+    return true;
+  },
+
+  /**
+   * @return {boolean}
+   * @private
+   */
+  shareIsEnabled_: function() {
+    if (this.configProperties_.Type == CrOnc.Type.WI_FI) {
+      var security = this.get('WiFi.Security', this.configProperties_);
+      if (!security || security == CrOnc.Security.NONE) {
+        return false;
+      } else if (security == CrOnc.Security.WPA_EAP) {
+        var outer = this.get('WiFi.EAP.Outer', this.configProperties_);
+        if (outer === 'EAP-TLS')
+          return false;
+      }
+      // TODO(stevenjb): Check certificates.
+    }
+    // TODO(stevenjb): Check login state.
+    return true;
+  },
+
+  /** @private */
+  onSaveTap_: function() {
+    var newConfig = /** @type {boolean} */ (!this.guid_);
+    var propertiesToSet = Object.assign({}, this.configProperties_);
+    if (newConfig) {
+      this.networkingPrivate.createNetwork(
+          this.shareNetwork_, propertiesToSet,
+          this.createNetworkCallback_.bind(this));
+    } else {
+      propertiesToSet.GUID = this.guid_;
+      this.networkingPrivate.setProperties(
+          this.guid_, propertiesToSet, this.setPropertiesCallback_.bind(this));
+    }
+  },
+
+  /**
+   * @param {string} guid
+   * @private
+   */
+  createNetworkCallback_: function(guid) {
+    var error = chrome.runtime.lastError && chrome.runtime.lastError.message;
+    if (error) {
+      // TODO(stevenjb): Display error message.
+      console.error(
+          'Error creating network type: ' + this.networkProperties_.Type +
+          ': ' + error);
+      return;
+    }
+    this.networkProperties_.GUID = guid;
+    this.fire('network-connect', {networkProperties: this.networkProperties_});
+    this.close_();
+  },
+
+  /** @private */
+  setPropertiesCallback_: function() {
+    var error = chrome.runtime.lastError && chrome.runtime.lastError.message;
+    if (error) {
+      console.error(
+          'Error setting network properties: ' + this.guid_ + ': ' + error);
+    }
+    this.close_();
+  },
+
+  /**
+   * @return boolean
+   * @private
+   */
+  showWiFiPassphrase_: function() {
+    if (this.configProperties_.Type != CrOnc.Type.WI_FI)
+      return false;
+    var security = this.get('WiFi.Security', this.configProperties_);
+    return security == CrOnc.Security.WEP_PSK ||
+        security == CrOnc.Security.WPA_PSK;
+  },
+
+  /** @private */
+  setShowEap_: function() {
+    var showEap = null;
+    if (this.configProperties_.Type == CrOnc.Type.WI_FI) {
+      var security = this.get('WiFi.Security', this.configProperties_);
+      if (security && security == CrOnc.Security.WPA_EAP) {

[michaelpg, 2017/05/18 21:14:23]

maybe consider early returns instead of nested ifs

[stevenjb, 2017/05/19 23:35:34]

Done.

+        var outer = this.get('WiFi.EAP.Outer', this.configProperties_);
+        showEap = {
+          Inner: outer == 'PEAP' || outer == 'EAP-TTLS',
+          ServerCA: outer != 'LEAP',
+          SubjectMatch: outer == 'EAP-TLS',
+          UserCert: outer == 'EAP-TLS',
+          Password: outer != 'EAP-TLS',
+          AnonymousIdentity: outer == 'PEAP' || outer == 'EAP-TTLS',
+        };
+      }
+    }
+    this.showEap_ = showEap;

[michaelpg, 2017/05/18 21:14:23]

er, is this property not declared/documented?

[stevenjb, 2017/05/19 23:35:34]

Done.

+  },
+
+  /**
+   * @param {string} key
+   * @param {string} prefix
+   * @return {string} The text to display for the onc value.
+   * @private
+   */
+  getOncLabel_: function(key, prefix) {
+    var oncKey = 'Onc' + prefix.replace(/\./g, '-') + '_' + key;
+    if (this.i18nExists(oncKey))
+      return this.i18n(oncKey);
+    return key;

[michaelpg, 2017/05/18 21:14:23]

Huh? Do we intentionally translate some strings but not others?

[stevenjb, 2017/05/19 23:35:34]

Added an assert.

+  },
+
+  /**
+   * @param {string} outer
+   * @return {!Array<string>}
+   * @private
+   */
+  getEapInnerItems_: function(outer) {
+    if (outer == 'PEAP')
+      return this.eapInnerItemsPeap_;
+    else if (outer == 'EAP-TTLS')

[michaelpg, 2017/05/18 21:14:23]

nit: no else after return

[stevenjb, 2017/05/19 23:35:34]

Done.

+      return this.eapInnerItemsTtls_;
+    return ['Automatic'];
+  },
+});