Show dummy values for all types of Passwords/Passphrases/PSK in case we have...

Show dummy values for all types of Passwords/Passphrases/PSK in case we have already stored a particular value


Change-Id: I6c6b8293aa7748152a44b945f092e563643328d8
Bug: 888959
Reviewed-on: https://chromium-review.googlesource.com/c/1296540
Commit-Queue: Roman Aleksandrov <raleksandrov@google.com>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607535}

chrome/test/data/extensions/api_test/networking_private/chromeos/test.js

@@ -685,6 +685,7 @@ var availableTests = [
             Passphrase: {
               Effective: 'UserSetting',
               UserEditable: true,
+              UserPolicy: 'FAKE_CREDENTIAL_VPaJDV9x',
               UserSetting: 'FAKE_CREDENTIAL_VPaJDV9x'
             },
             SSID: {

chromeos/network/onc/onc_merger.cc

@@ -14,6 +14,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/values.h"
 #include "chromeos/network/onc/onc_signature.h"
+#include "chromeos/network/policy_util.h"
 #include "components/onc/onc_constants.h"
 
 namespace chromeos {
@@ -421,11 +422,29 @@ class MergeToAugmented : public MergeToEffective {
                               base::Value(which_effective));
     }
 
-    // Prevent credentials from being forwarded in cleartext to
-    // UI. User/shared credentials are not stored separately, so they cannot
+    // Prevent credentials from being forwarded in cleartext to UI.
+    // User/shared credentials are not stored separately, so they cannot
     // leak here.
+    // User and Shared settings are already replaced with |kFakeCredential|.
     bool is_credential = onc::FieldIsCredential(*signature_, key);
-    if (!is_credential) {
+    if (is_credential) {
+      // Set |kFakeCredential| to notify UI that credential is saved.
+      if (values.user_policy) {
+        augmented_value->SetKey(
+            ::onc::kAugmentationUserPolicy,
+            base::Value(chromeos::policy_util::kFakeCredential));
+      }
+      if (values.device_policy) {
+        augmented_value->SetKey(
+            ::onc::kAugmentationDevicePolicy,
+            base::Value(chromeos::policy_util::kFakeCredential));
+      }
+      if (values.active_setting) {
+        augmented_value->SetKey(
+            ::onc::kAugmentationActiveSetting,
+            base::Value(chromeos::policy_util::kFakeCredential));
+      }
+    } else {
       if (values.user_policy) {
         augmented_value->SetKey(::onc::kAugmentationUserPolicy,
                                 values.user_policy->Clone());

chromeos/network/onc/onc_translator_shill_to_onc.cc

@@ -310,14 +310,6 @@ void ShillToONCTranslator::TranslateIPsec() {
   }
   onc_object_->SetKey(::onc::ipsec::kAuthenticationType,
                       base::Value(authentication_type));
-  if (authentication_type == ::onc::ipsec::kPSK) {
-    // If L2TPIPsec.PSKRequired is false, provide an empty PSK value to indicate
-    // that the PSK is saved but not known.
-    const base::Value* psk_required = shill_dictionary_->FindKeyOfType(
-        shill::kL2tpIpsecPskRequiredProperty, base::Value::Type::BOOLEAN);
-    if (psk_required && !psk_required->GetBool())
-      onc_object_->SetKey(::onc::ipsec::kPSK, base::Value(""));
-  }
 }
 
 void ShillToONCTranslator::TranslateThirdPartyVPN() {

chromeos/network/policy_util.cc

@@ -24,12 +24,9 @@ namespace chromeos {
 
 namespace policy_util {
 
-namespace {
-
-// This fake credential contains a random postfix which is extremely unlikely to
-// be used by any user.
 const char kFakeCredential[] = "FAKE_CREDENTIAL_VPaJDV9x";
 
+namespace {
 
 // Removes all kFakeCredential values from sensitive fields (determined by
 // onc::FieldIsCredential) of |onc_object|.

chromeos/network/policy_util.h

@@ -19,6 +19,11 @@ struct NetworkProfile;
 
 namespace policy_util {
 
+// This fake credential contains a random postfix which is extremely unlikely to
+// be used by any user. Used to determine saved but unknown credential
+// (PSK/Passphrase/Password) in UI (see network_password_input.js).
+extern const char kFakeCredential[];
+
 using GuidToPolicyMap =
     std::map<std::string, std::unique_ptr<base::DictionaryValue>>;
 

chromeos/test/data/network/augmented_merge.json

@@ -75,7 +75,9 @@
             "UserPolicy": 1
          },
          "PSK": {
-            "Effective": "UserPolicy"
+          "DevicePolicy": "FAKE_CREDENTIAL_VPaJDV9x",
+          "Effective": "UserPolicy",
+          "UserPolicy": "FAKE_CREDENTIAL_VPaJDV9x"
          }
       },
       "OpenVPN": {

ui/webui/resources/cr_components/chromeos/network/network_config.html

@@ -74,8 +74,6 @@
         <template is="dom-if" if="[[!showVpn_.Cert]]">
           <network-password-input label="[[i18n('OncVPN-IPsec-PSK')]]"
               value="{{configProperties_.VPN.IPsec.PSK}}"
-              on-focus="onPskFocus_" on-blur="onPskBlur_"
-              on-input="onPskInput_"
               property="[[managedProperties.VPN.IPsec.PSK]]">
           </network-password-input>
         </template>

ui/webui/resources/cr_components/chromeos/network/network_config.js

@@ -27,9 +27,6 @@ const VPNConfigType = {
 /** @type {string}  */ const NO_CERTS_HASH = 'no-certs';
 /** @type {string}  */ const NO_USER_CERT_HASH = 'no-user-cert';
 
-// Used to indicate a saved but unknown PSK value. Will appear as *'s in the
-// PSK field by default.
-/** @type {string}  */ const UNKNOWN_PSK = '        ';
 
 Polymer({
   is: 'network-config',
@@ -159,15 +156,6 @@ Polymer({
     /** @private {string|undefined} */
     selectedUserCertHash_: String,
 
-    /**
-     * Set to true when the PSK is saved but the value is unknown.
-     * @private
-     */
-    pskSavedUnknown_: {
-      type: Boolean,
-      value: false,
-    },
-
     /**
      * Whether all required properties have been set.
      * @private
@@ -556,58 +544,10 @@ Polymer({
           !!CrOnc.getActiveValue(
               /** @type {chrome.networkingPrivate.ManagedBoolean|undefined} */
               (this.get('VPN.L2TP.SaveCredentials', managedProperties)));
-      if (CrOnc.getActiveValue(
-              /** @type {chrome.networkingPrivate.ManagedDOMString|undefined} */
-              (this.get('VPN.IPsec.PSK', managedProperties))) === '') {
-        // If an empty PSK is provided, show a blank value in the UI to indicate
-        // that the PSK has a saved value.
-        this.pskSavedUnknown_ = true;
-      } else {
-        this.pskSavedUnknown_ = false;
-      }
     }
-    this.setManagedProperties_(managedProperties);
-  },
 
-  /**
-   * If the IPsec.PSK field is focused and the PSK value is saved but unknown,
-   * clear the pseudo value set in getPropertiesCallback_.
-   * @param {!InputEvent} e
-   * @private
-   */
-  onPskFocus_: function(e) {
-    if (this.pskSavedUnknown_) {
-      // We can not rely on data binding to update the target value when a
-      // field is focused.
-      e.target.value = '';
-      this.set('VPN.IPsec.PSK', '', this.configProperties_);
-    }
-  },
-
-  /**
-   * If the IPsec.PSK field is in the saved-but-unknown state, restore the
-   * pseudo value when the field is unfocused.
-   * @param {!InputEvent} e
-   * @private
-   */
-  onPskBlur_: function(e) {
-    if (this.pskSavedUnknown_) {
-      // The target is still focused so we can not rely on data binding to
-      // update the target value.
-      e.target.value = UNKNOWN_PSK;
-      this.set('VPN.IPsec.PSK', UNKNOWN_PSK, this.configProperties_);
-    }
-  },
-
-  /**
-   * When the IPsec.PSK field is changed, clear pskSavedUnknown_.
-   * @param {!InputEvent} e
-   * @private
-   */
-  onPskInput_: function(e) {
-    this.pskSavedUnknown_ = false;
+    this.setManagedProperties_(managedProperties);
   },
-
   /**
    * @param {!chrome.networkingPrivate.ManagedProperties} managedProperties
    * @private
@@ -794,9 +734,6 @@ Polymer({
                         {AuthenticationType: CrOnc.IPsecAuthenticationType.PSK},
                         CrOnc.getActiveProperties(
                             managedProperties.VPN.IPsec)));
-            if (this.pskSavedUnknown_) {
-              this.set('IPsec.PSK', UNKNOWN_PSK, vpn);
-            }
             vpn.L2TP = Object.assign(
                 {Username: ''},
                 CrOnc.getActiveProperties(managedProperties.VPN.L2TP));
@@ -1318,8 +1255,7 @@ Polymer({
 
     switch (this.vpnType_) {
       case VPNConfigType.L2TP_IPSEC_PSK:
-        return !!this.get('L2TP.Username', vpn) &&
-            (this.pskSavedUnknown_ || !!this.get('IPsec.PSK', vpn));
+        return !!this.get('L2TP.Username', vpn) && !!this.get('IPsec.PSK', vpn);
       case VPNConfigType.L2TP_IPSEC_CERT:
         return !!this.get('L2TP.Username', vpn) &&
             this.selectedUserCertHashIsValid_();
@@ -1416,7 +1352,6 @@ Polymer({
     }
 
     openvpn.SaveCredentials = this.vpnSaveCredentials_;
-
     propertiesToSet.VPN.OpenVPN = openvpn;
   },
 
@@ -1435,8 +1370,6 @@ Polymer({
     vpn.IPsec.IKEVersion = 1;
     vpn.IPsec.SaveCredentials = this.vpnSaveCredentials_;
     vpn.L2TP.SaveCredentials = this.vpnSaveCredentials_;
-    if (this.pskSavedUnknown_)
-      delete vpn.IPsec.PSK;
   },
 
   /**

ui/webui/resources/cr_components/chromeos/network/network_password_input.html

@@ -30,12 +30,15 @@
     <div id="container">
       <cr-input id="input" label="[[label]]" value="{{value}}"
           disabled="[[getDisabled_(disabled, property)]]"
-          type="[[getInputType_(showPassword)]]" on-keypress="onInputKeypress_">
+          type="[[getInputType_(showPassword)]]"
+          on-focus="onFocus_" on-blur="onBlur_" on-input="onInput_"
+          on-keypress="onKeypress_">
       </cr-input>
       <template is="dom-if" if="[[!showPolicyIndicator_]]" restamp>
         <paper-icon-button-light id="icon" slot="suffix"
             class$="[[getIconClass_(showPassword)]]">
           <button on-tap="onShowPasswordTap_"
+              disabled="[[getButtonDisabled_(value)]]"
               title="[[getShowPasswordTitle_(showPassword)]]">
           </button>
         </paper-icon-button-light>

ui/webui/resources/cr_components/chromeos/network/network_password_input.js

@@ -5,6 +5,12 @@
 /**
  * @fileoverview Polymer element for network password input fields.
  */
+
+// Used to indicate a saved but unknown credential value. Will appear as *'s in
+// the credential (passphrase, password, etc.) field by default.
+// See |kFakeCredential| in chromeos/network/policy_util.h.
+/** @type {string} */ const FAKE_CREDENTIAL = 'FAKE_CREDENTIAL_VPaJDV9x';
+
 Polymer({
   is: 'network-password-input',
 
@@ -30,12 +36,27 @@ Polymer({
       value: false,
     },
 
-    /** @private */
     showPolicyIndicator_: {
       type: Boolean,
       value: false,
       computed: 'getDisabled_(disabled, property)',
     },
+
+    /** @private */
+    restoreUnknown_: {
+      type: Boolean,
+      value: false,
+    },
+  },
+
+  observers: [
+    'updateShowPassword_(value)',
+  ],
+
+  /** @private */
+  updateShowPassword_: function() {
+    if (this.value == FAKE_CREDENTIAL)
+      this.showPassword = false;
   },
 
   focus: function() {
@@ -67,6 +88,17 @@ Polymer({
                                this.i18n('showPassword');
   },
 
+  /**
+   * @param {string}  value
+   * @return {boolean} True if the value equals |FAKE_CREDENTIAL| to
+   * prevent users from seeing this fake credential, but they should be able to
+   * see their custom input.
+   * @private
+   */
+  getButtonDisabled_: function(value) {
+    return value == FAKE_CREDENTIAL;
+  },
+
   /**
    * @param {!Event} event
    * @private
@@ -80,10 +112,54 @@ Polymer({
    * @param {!Event} event
    * @private
    */
-  onInputKeypress_: function(event) {
+  onKeypress_: function(event) {
     if (event.target.id != 'input' || event.key != 'Enter')
       return;
     event.stopPropagation();
     this.fire('enter');
   },
+
+  /**
+   * If the input field is focused and the value is |FAKE_CREDENTIAL|,
+   * clear the value.
+   * @param {!InputEvent} e
+   * @private
+   */
+  onFocus_: function(e) {
+    if (this.value != FAKE_CREDENTIAL)
+      return;
+    // We can not rely on data binding to update the target value when a
+    // field is focused.
+    e.target.value = '';
+    this.value = '';
+    // Remember to restore |FAKE_CREDENTIAL| if the user doesn't change
+    // the input value.
+    this.restoreUnknown_ = true;
+  },
+
+  /**
+   * If the input field should be restored, restore the |FAKE_CREDENTIAL|
+   * value when the field is unfocused.
+   * @param {!InputEvent} e
+   * @private
+   */
+  onBlur_: function(e) {
+    if (!this.restoreUnknown_)
+      return;
+    // The target is still focused so we can not rely on data binding to
+    // update the target value.
+    e.target.value = FAKE_CREDENTIAL;
+    this.value = FAKE_CREDENTIAL;
+  },
+
+  /**
+   * When the input field is changed, clear |restoreUnknown_|.
+   * @param {!InputEvent} e
+   * @private
+   */
+  onInput_: function(e) {
+    this.restoreUnknown_ = false;
+  },
+
+
 });