summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Eastwood <contact@ericeastwood.com>2017-12-13 16:07:47 -0600
committerEric Eastwood <contact@ericeastwood.com>2017-12-15 14:06:55 -0600
commit5e7d1878cb27885f6453cafed2d978628fb5535c (patch)
treee10055f5b468b42da2bd7fbb625b45eaaaac07ae
parent627a96875ee68e37b45192af3121f09032ea4bbf (diff)
downloadgitlab-ce-5e7d1878cb27885f6453cafed2d978628fb5535c.tar.gz
Hide runner token in CI/CD settings page38019-hide-runner-token
Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/38019
-rw-r--r--app/assets/javascripts/behaviors/secret_values.js42
-rw-r--r--app/assets/javascripts/dispatcher.js14
-rw-r--r--app/assets/javascripts/project_variables.js39
-rw-r--r--app/views/ci/variables/_index.html.haml6
-rw-r--r--app/views/ci/variables/_table.html.haml6
-rw-r--r--app/views/projects/pipelines_settings/_show.html.haml8
-rw-r--r--changelogs/unreleased/38019-hide-runner-token.yml5
-rw-r--r--spec/features/group_variables_spec.rb2
-rw-r--r--spec/features/variables_spec.rb4
-rw-r--r--spec/javascripts/behaviors/secret_values_spec.js146
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);
+ });
+ });
+ });
+});