From ae9eddb002b21e514cfdd41dc11df05bd476eec6 Mon Sep 17 00:00:00 2001
From: Maja Komel <maja.komel@gmail.com>
Date: Sun, 4 Nov 2018 21:18:58 +0100
Subject: [PATCH] FIX: don't allow adding a value containing vertical bar char
 to the secret list

---
 .../admin/components/secret-value-list.js.es6 | 21 ++++++++++++--
 .../components/secret-value-list.hbs          |  2 ++
 .../stylesheets/common/admin/admin_base.scss  | 23 +++++++++++----
 config/locales/client.en.yml                  |  2 ++
 .../components/secret-value-list-test.js.es6  | 28 +++++++++++++++++++
 5 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/app/assets/javascripts/admin/components/secret-value-list.js.es6 b/app/assets/javascripts/admin/components/secret-value-list.js.es6
index 939ba45c9e7..26e9f93e280 100644
--- a/app/assets/javascripts/admin/components/secret-value-list.js.es6
+++ b/app/assets/javascripts/admin/components/secret-value-list.js.es6
@@ -2,11 +2,10 @@ import { on } from "ember-addons/ember-computed-decorators";
 
 export default Ember.Component.extend({
   classNameBindings: [":value-list", ":secret-value-list"],
-  inputInvalidKey: Ember.computed.empty("newKey"),
-  inputInvalidSecret: Ember.computed.empty("newSecret"),
   inputDelimiter: null,
   collection: null,
   values: null,
+  validationMessage: null,
 
   @on("didReceiveAttrs")
   _setupCollection() {
@@ -20,15 +19,18 @@ export default Ember.Component.extend({
 
   actions: {
     changeKey(index, newValue) {
+      if (this._checkInvalidInput(newValue)) return;
       this._replaceValue(index, newValue, "key");
     },
 
     changeSecret(index, newValue) {
+      if (this._checkInvalidInput(newValue)) return;
       this._replaceValue(index, newValue, "secret");
     },
 
     addValue() {
-      if (this.get("inputInvalidKey") || this.get("inputInvalidSecret")) return;
+      if (this._checkInvalidInput([this.get("newKey"), this.get("newSecret")]))
+        return;
       this._addValue(this.get("newKey"), this.get("newSecret"));
       this.setProperties({ newKey: "", newSecret: "" });
     },
@@ -38,6 +40,19 @@ export default Ember.Component.extend({
     }
   },
 
+  _checkInvalidInput(inputs) {
+    this.set("validationMessage", null);
+    for (let input of inputs) {
+      if (Ember.isEmpty(input) || input.includes("|")) {
+        this.set(
+          "validationMessage",
+          I18n.t("admin.site_settings.secret_list.invalid_input")
+        );
+        return true;
+      }
+    }
+  },
+
   _addValue(value, secret) {
     this.get("collection").addObject({ key: value, secret: secret });
     this._saveValues();
diff --git a/app/assets/javascripts/admin/templates/components/secret-value-list.hbs b/app/assets/javascripts/admin/templates/components/secret-value-list.hbs
index 6bdc972ddc1..a2f44f15530 100644
--- a/app/assets/javascripts/admin/templates/components/secret-value-list.hbs
+++ b/app/assets/javascripts/admin/templates/components/secret-value-list.hbs
@@ -20,3 +20,5 @@
              icon="plus"
              class="add-value-btn btn-small"}}
 </div>
+
+{{setting-validation-message message=validationMessage}}
diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss
index 24721a26be6..e8df33fb5f4 100644
--- a/app/assets/stylesheets/common/admin/admin_base.scss
+++ b/app/assets/stylesheets/common/admin/admin_base.scss
@@ -935,12 +935,23 @@ table#user-badges {
       margin-left: 0.25em;
       margin-top: 0.125em;
     }
-    &:last-of-type {
-      .new-value-input {
-        &:first-of-type {
-          margin-left: 0.25em;
-        }
-      }
+    .new-value-input {
+      margin-left: 0.25em;
+    }
+  }
+}
+
+.mobile-view .secret-value-list {
+  .add-value-btn {
+    margin-bottom: 9px;
+  }
+  .value {
+    .value-input:last-of-type {
+      margin-left: 2.35em;
+    }
+    .new-value-input:first-of-type {
+      margin-right: 2.15em;
+      margin-left: 0.25em;
     }
   }
 }
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index a12320dbc6b..b7829bb98a7 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -3948,6 +3948,8 @@ en:
           tags: "Tags"
           search: "Search"
           groups: "Groups"
+        secret_list:
+          invalid_input: "Input fields cannot be empty or contain vertical bar character."
 
       badges:
         title: Badges
diff --git a/test/javascripts/components/secret-value-list-test.js.es6 b/test/javascripts/components/secret-value-list-test.js.es6
index 1f602cbdbde..a55d2893061 100644
--- a/test/javascripts/components/secret-value-list-test.js.es6
+++ b/test/javascripts/components/secret-value-list-test.js.es6
@@ -41,6 +41,34 @@ componentTest("adding a value", {
   }
 });
 
+componentTest("adding an invalid value", {
+  template: "{{secret-value-list values=values}}",
+
+  async test(assert) {
+    await fillIn(".new-value-input.key", "someString");
+    await fillIn(".new-value-input.secret", "keyWithAPipe|Hidden");
+    await click(".add-value-btn");
+
+    assert.ok(
+      find(".values .value").length === 0,
+      "it doesn't add the value to the list of values"
+    );
+
+    assert.deepEqual(
+      this.get("values"),
+      undefined,
+      "it doesn't add the value to the list of values"
+    );
+
+    assert.ok(
+      find(".validation-error")
+        .html()
+        .indexOf(I18n.t("admin.site_settings.secret_list.invalid_input")) > -1,
+      "it shows validation error"
+    );
+  }
+});
+
 componentTest("removing a value", {
   template: "{{secret-value-list values=values}}",