Hide runner token in CI/CD settings page38019-hide-runner-token

Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/38019

10 files changed, 223 insertions, 49 deletions

diff --git a/app/assets/javascripts/behaviors/secret_values.js b/app/assets/javascripts/behaviors/secret_values.js
new file mode 100644
index 00000000000..1cf0b960eb0
--- /dev/null
+++ b/app/assets/javascripts/behaviors/secret_values.js
@@ -0,0 +1,42 @@
+import { n__ } from '../locale';
+import { convertPermissionToBoolean } from '../lib/utils/common_utils';
+
+export default class SecretValues {
+  constructor(container) {
+    this.container = container;
+  }
+
+  init() {
+    this.values = this.container.querySelectorAll('.js-secret-value');
+    this.placeholders = this.container.querySelectorAll('.js-secret-value-placeholder');
+    this.revealButton = this.container.querySelector('.js-secret-value-reveal-button');
+
+    this.revealText = n__('Reveal value', 'Reveal values', this.values.length);
+    this.hideText = n__('Hide value', 'Hide values', this.values.length);
+
+    const isRevealed = convertPermissionToBoolean(this.revealButton.dataset.secretRevealStatus);
+    this.updateDom(isRevealed);
+
+    this.revealButton.addEventListener('click', this.onRevealButtonClicked.bind(this));
+  }
+
+  onRevealButtonClicked() {
+    const previousIsRevealed = convertPermissionToBoolean(
+      this.revealButton.dataset.secretRevealStatus,
+    );
+    this.updateDom(!previousIsRevealed);
+  }
+
+  updateDom(isRevealed) {
+    this.values.forEach((value) => {
+      value.classList.toggle('hide', !isRevealed);
+    });
+
+    this.placeholders.forEach((placeholder) => {
+      placeholder.classList.toggle('hide', isRevealed);
+    });
+
+    this.revealButton.textContent = isRevealed ? this.hideText : this.revealText;
+    this.revealButton.dataset.secretRevealStatus = isRevealed;
+  }
+}
diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index 5721375f4c6..f62a0208110 100644
--- a/app/assets/javascripts/dispatcher.js
+++ b/app/assets/javascripts/dispatcher.js
@@ -35,6 +35,7 @@ import Flash from './flash';
 import CommitsList from './commits';
 import Issue from './issue';
 import BindInOut from './behaviors/bind_in_out';
+import SecretValues from './behaviors/secret_values';
 import DeleteModal from './branches/branches_delete_modal';
 import Group from './group';
 import GroupsList from './groups_list';
@@ -90,7 +91,6 @@ import memberExpirationDate from './member_expiration_date';
 import DueDateSelectors from './due_date_select';
 import Diff from './diff';
 import ProjectLabelSubscription from './project_label_subscription';
-import ProjectVariables from './project_variables';
 import SearchAutocomplete from './search_autocomplete';
 import Activities from './activities';
 
@@ -527,8 +527,18 @@ import Activities from './activities';
         case 'projects:settings:ci_cd:show':
           // Initialize expandable settings panels
           initSettingsPanels();
+
+          const runnerToken = document.querySelector('.js-secret-runner-token');
+          if (runnerToken) {
+            const runnerTokenSecretValue = new SecretValues(runnerToken);
+            runnerTokenSecretValue.init();
+          }
         case 'groups:settings:ci_cd:show':
-          new ProjectVariables();
+          const secretVariableTable = document.querySelector('.js-secret-variable-table');
+          if (secretVariableTable) {
+            const secretVariableTableValues = new SecretValues(secretVariableTable);
+            secretVariableTableValues.init();
+          }
           break;
         case 'ci:lints:create':
         case 'ci:lints:show':
diff --git a/app/assets/javascripts/project_variables.js b/app/assets/javascripts/project_variables.js
deleted file mode 100644
index 567c311f119..00000000000
--- a/app/assets/javascripts/project_variables.js
+++ /dev/null
@@ -1,39 +0,0 @@
-
-const HIDDEN_VALUE_TEXT = '******';
-
-export default class ProjectVariables {
-  constructor() {
-    this.$revealBtn = $('.js-btn-toggle-reveal-values');
-    this.$revealBtn.on('click', this.toggleRevealState.bind(this));
-  }
-
-  toggleRevealState(e) {
-    e.preventDefault();
-
-    const oldStatus = this.$revealBtn.attr('data-status');
-    let newStatus = 'hidden';
-    let newAction = 'Reveal Values';
-
-    if (oldStatus === 'hidden') {
-      newStatus = 'revealed';
-      newAction = 'Hide Values';
-    }
-
-    this.$revealBtn.attr('data-status', newStatus);
-
-    const $variables = $('.variable-value');
-
-    $variables.each((_, variable) => {
-      const $variable = $(variable);
-      let newText = HIDDEN_VALUE_TEXT;
-
-      if (newStatus === 'revealed') {
-        newText = $variable.attr('data-value');
-      }
-
-      $variable.text(newText);
-    });
-
-    this.$revealBtn.text(newAction);
-  }
-}
diff --git a/app/views/ci/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml
index 2bac69bc536..6e399fc7392 100644
--- a/app/views/ci/variables/_index.html.haml
+++ b/app/views/ci/variables/_index.html.haml
@@ -10,5 +10,7 @@
       %p.settings-message.text-center.append-bottom-0
         No variables found, add one with the form above.
     - else
-      = render "ci/variables/table"
-      %button.btn.btn-info.js-btn-toggle-reveal-values{ "data-status" => 'hidden' } Reveal Values
+      .js-secret-variable-table
+        = render "ci/variables/table"
+        %button.btn.btn-info.js-secret-value-reveal-button{ data: { secret_reveal_status: 'false' } }
+          = n_('Reveal value', 'Reveal values', @variables.size)
diff --git a/app/views/ci/variables/_table.html.haml b/app/views/ci/variables/_table.html.haml
index 71a0b56c4f4..2298930d0c7 100644
--- a/app/views/ci/variables/_table.html.haml
+++ b/app/views/ci/variables/_table.html.haml
@@ -15,7 +15,11 @@
         - if variable.id?
           %tr
             %td.variable-key= variable.key
-            %td.variable-value{ "data-value" => variable.value }******
+            %td.variable-value
+              %span.js-secret-value-placeholder
+                = '*' * 6
+              %span.hide.js-secret-value
+                = variable.value
             %td.variable-protected= Gitlab::Utils.boolean_to_yes_no(variable.protected)
             %td.variable-menu
               = link_to variable.edit_path, class: "btn btn-transparent btn-variable-edit" do
diff --git a/app/views/projects/pipelines_settings/_show.html.haml b/app/views/projects/pipelines_settings/_show.html.haml
index c63e716180c..c5f9f5aa15b 100644
--- a/app/views/projects/pipelines_settings/_show.html.haml
+++ b/app/views/projects/pipelines_settings/_show.html.haml
@@ -40,10 +40,14 @@
             = form.text_field :domain, class: 'form-control', placeholder: 'domain.com'
 
         %hr
-        .form-group.append-bottom-default
+        .form-group.append-bottom-default.js-secret-runner-token
           = f.label :runners_token, "Runner token", class: 'label-light'
-          = f.text_field :runners_token, class: "form-control", placeholder: 'xEeFCaDAB89'
+          .form-control.js-secret-value-placeholder
+            = '*' * 20
+          = f.text_field :runners_token, class: "form-control hide js-secret-value", placeholder: 'xEeFCaDAB89'
           %p.help-block The secure token used by the Runner to checkout the project
+          %button.btn.btn-info.prepend-top-10.js-secret-value-reveal-button{ type: 'button', data: { secret_reveal_status: 'false' } }
+            = _('Reveal value')
 
         %hr
         .form-group
diff --git a/changelogs/unreleased/38019-hide-runner-token.yml b/changelogs/unreleased/38019-hide-runner-token.yml
new file mode 100644
index 00000000000..11ae0a685ef
--- /dev/null
+++ b/changelogs/unreleased/38019-hide-runner-token.yml
@@ -0,0 +1,5 @@
+---
+title: Hide runner token in CI/CD settings page
+merge_request:
+author:
+type: added
diff --git a/spec/features/group_variables_spec.rb b/spec/features/group_variables_spec.rb
index d2d0be35f1c..e9b375f4c94 100644
--- a/spec/features/group_variables_spec.rb
+++ b/spec/features/group_variables_spec.rb
@@ -24,7 +24,7 @@ feature 'Group variables', :js do
         expect(find(".variable-value")).to have_content('******')
         expect(find(".variable-protected")).to have_content('Yes')
       end
-      click_on 'Reveal Values'
+      click_on 'Reveal value'
       page.within('.variables-table') do
         expect(find(".variable-value")).to have_content('AAA123')
       end
diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb
index c78f7d0d9be..dde60c83536 100644
--- a/spec/features/variables_spec.rb
+++ b/spec/features/variables_spec.rb
@@ -65,14 +65,14 @@ describe 'Project variables', :js do
       expect(page).to have_content('******')
     end
 
-    click_button('Reveal Values')
+    click_button('Reveal values')
 
     page.within('.variables-table') do
       expect(page).to have_content('key')
       expect(page).to have_content('key value')
     end
 
-    click_button('Hide Values')
+    click_button('Hide values')
 
     page.within('.variables-table') do
       expect(page).to have_content('key')
diff --git a/spec/javascripts/behaviors/secret_values_spec.js b/spec/javascripts/behaviors/secret_values_spec.js
new file mode 100644
index 00000000000..9eeae474e7d
--- /dev/null
+++ b/spec/javascripts/behaviors/secret_values_spec.js
@@ -0,0 +1,146 @@
+import SecretValues from '~/behaviors/secret_values';
+
+function generateFixtureMarkup(secrets, isRevealed) {
+  return `
+  <div class="js-secret-container">
+    ${secrets.map(secret => `
+      <div class="js-secret-value-placeholder">
+        ***
+      </div>
+      <div class="hide js-secret-value">
+        ${secret}
+      </div>
+    `).join('')}
+    <button
+      class="js-secret-value-reveal-button"
+      data-secret-reveal-status="${isRevealed}"
+    >
+      ...
+    </button>
+  </div>
+  `;
+}
+
+function setupSecretFixture(secrets, isRevealed) {
+  const wrapper = document.createElement('div');
+  wrapper.innerHTML = generateFixtureMarkup(secrets, isRevealed);
+
+  const secretValues = new SecretValues(wrapper.querySelector('.js-secret-container'));
+  secretValues.init();
+
+  return wrapper;
+}
+
+describe('setupSecretValues', () => {
+  describe('with a single secret', () => {
+    const secrets = ['mysecret123'];
+
+    it('should have correct "Reveal" label when values are hidden', () => {
+      const wrapper = setupSecretFixture(secrets, false);
+      const revealButton = wrapper.querySelector('.js-secret-value-reveal-button');
+
+      expect(revealButton.textContent).toEqual('Reveal value');
+    });
+
+    it('should have correct "Hide" label when values are shown', () => {
+      const wrapper = setupSecretFixture(secrets, true);
+      const revealButton = wrapper.querySelector('.js-secret-value-reveal-button');
+
+      expect(revealButton.textContent).toEqual('Hide value');
+    });
+
+    it('should value hidden initially', () => {
+      const wrapper = setupSecretFixture(secrets, false);
+      const values = wrapper.querySelectorAll('.js-secret-value');
+      const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder');
+
+      expect(values.length).toEqual(1);
+      expect(values[0].classList.contains('hide')).toEqual(true);
+      expect(placeholders.length).toEqual(1);
+      expect(placeholders[0].classList.contains('hide')).toEqual(false);
+    });
+
+    it('should toggle value and placeholder', () => {
+      const wrapper = setupSecretFixture(secrets, false);
+      const revealButton = wrapper.querySelector('.js-secret-value-reveal-button');
+      const values = wrapper.querySelectorAll('.js-secret-value');
+      const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder');
+
+      revealButton.click();
+
+      expect(values.length).toEqual(1);
+      expect(values[0].classList.contains('hide')).toEqual(false);
+      expect(placeholders.length).toEqual(1);
+      expect(placeholders[0].classList.contains('hide')).toEqual(true);
+
+      revealButton.click();
+
+      expect(values.length).toEqual(1);
+      expect(values[0].classList.contains('hide')).toEqual(true);
+      expect(placeholders.length).toEqual(1);
+      expect(placeholders[0].classList.contains('hide')).toEqual(false);
+    });
+  });
+
+  describe('with a multiple secrets', () => {
+    const secrets = ['mysecret123', 'happygoat456', 'tanuki789'];
+
+    it('should have correct "Reveal" label when values are hidden', () => {
+      const wrapper = setupSecretFixture(secrets, false);
+      const revealButton = wrapper.querySelector('.js-secret-value-reveal-button');
+
+      expect(revealButton.textContent).toEqual('Reveal values');
+    });
+
+    it('should have correct "Hide" label when values are shown', () => {
+      const wrapper = setupSecretFixture(secrets, true);
+      const revealButton = wrapper.querySelector('.js-secret-value-reveal-button');
+
+      expect(revealButton.textContent).toEqual('Hide values');
+    });
+
+    it('should have all values hidden initially', () => {
+      const wrapper = setupSecretFixture(secrets, false);
+      const values = wrapper.querySelectorAll('.js-secret-value');
+      const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder');
+
+      expect(values.length).toEqual(3);
+      values.forEach((value) => {
+        expect(value.classList.contains('hide')).toEqual(true);
+      });
+      expect(placeholders.length).toEqual(3);
+      placeholders.forEach((placeholder) => {
+        expect(placeholder.classList.contains('hide')).toEqual(false);
+      });
+    });
+
+    it('should toggle values and placeholders', () => {
+      const wrapper = setupSecretFixture(secrets, false);
+      const revealButton = wrapper.querySelector('.js-secret-value-reveal-button');
+      const values = wrapper.querySelectorAll('.js-secret-value');
+      const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder');
+
+      revealButton.click();
+
+      expect(values.length).toEqual(3);
+      values.forEach((value) => {
+        expect(value.classList.contains('hide')).toEqual(false);
+      });
+      expect(placeholders.length).toEqual(3);
+      placeholders.forEach((placeholder) => {
+        expect(placeholder.classList.contains('hide')).toEqual(true);
+      });
+
+      revealButton.click();
+
+      expect(values.length).toEqual(3);
+      values.forEach((value) => {
+        expect(value.classList.contains('hide')).toEqual(true);
+      });
+      expect(placeholders.length).toEqual(3);
+      placeholders.forEach((placeholder) => {
+        expect(placeholder.classList.contains('hide')).toEqual(false);
+      });
+    });
+  });
+});