diff options
author | Eric Eastwood <contact@ericeastwood.com> | 2017-12-14 01:07:47 +0300 |
---|---|---|
committer | Eric Eastwood <contact@ericeastwood.com> | 2017-12-15 23:06:55 +0300 |
commit | 5e7d1878cb27885f6453cafed2d978628fb5535c (patch) | |
tree | e10055f5b468b42da2bd7fbb625b45eaaaac07ae | |
parent | 627a96875ee68e37b45192af3121f09032ea4bbf (diff) |
Hide runner token in CI/CD settings page
Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/38019
-rw-r--r-- | app/assets/javascripts/behaviors/secret_values.js | 42 | ||||
-rw-r--r-- | app/assets/javascripts/dispatcher.js | 14 | ||||
-rw-r--r-- | app/assets/javascripts/project_variables.js | 39 | ||||
-rw-r--r-- | app/views/ci/variables/_index.html.haml | 6 | ||||
-rw-r--r-- | app/views/ci/variables/_table.html.haml | 6 | ||||
-rw-r--r-- | app/views/projects/pipelines_settings/_show.html.haml | 8 | ||||
-rw-r--r-- | changelogs/unreleased/38019-hide-runner-token.yml | 5 | ||||
-rw-r--r-- | spec/features/group_variables_spec.rb | 2 | ||||
-rw-r--r-- | spec/features/variables_spec.rb | 4 | ||||
-rw-r--r-- | spec/javascripts/behaviors/secret_values_spec.js | 146 |
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); + }); + }); + }); +}); |