Hide CI secret variable values on save

See https://gitlab.com/gitlab-org/gitlab-ce/issues/42928
This commit is contained in:
Eric Eastwood 2018-02-09 13:09:33 -06:00
parent dbb934c8e2
commit c6b2ff8a21
5 changed files with 82 additions and 15 deletions

View File

@ -75,6 +75,7 @@ export default class AjaxVariableList {
if (res.status === statusCodes.OK && res.data) {
this.updateRowsWithPersistedVariables(res.data.variables);
this.variableList.hideValues();
} else if (res.status === statusCodes.BAD_REQUEST) {
// Validation failed
this.errorBox.innerHTML = generateErrorBoxContent(res.data);

View File

@ -178,6 +178,10 @@ export default class VariableList {
this.$container.find('.js-row-remove-button').attr('disabled', !isEnabled);
}
hideValues() {
this.secretValues.updateDom(false);
}
getAllData() {
// Ignore the last empty row because we don't want to try persist
// a blank variable and run into validation problems.

View File

@ -0,0 +1,5 @@
---
title: Hide CI secret variable values after saving
merge_request: 17044
author:
type: changed

View File

@ -1,8 +1,10 @@
import $ from 'jquery';
import MockAdapter from 'axios-mock-adapter';
import axios from '~/lib/utils/axios_utils';
import AjaxFormVariableList from '~/ci_variable_list/ajax_variable_list';
const VARIABLE_PATCH_ENDPOINT = 'http://test.host/frontend-fixtures/builds-project/variables';
const HIDE_CLASS = 'hide';
describe('AjaxFormVariableList', () => {
preloadFixtures('projects/ci_cd_settings.html.raw');
@ -45,16 +47,16 @@ describe('AjaxFormVariableList', () => {
const loadingIcon = saveButton.querySelector('.js-secret-variables-save-loading-icon');
mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => {
expect(loadingIcon.classList.contains('hide')).toEqual(false);
expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(false);
return [200, {}];
});
expect(loadingIcon.classList.contains('hide')).toEqual(true);
expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true);
ajaxVariableList.onSaveClicked()
.then(() => {
expect(loadingIcon.classList.contains('hide')).toEqual(true);
expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true);
})
.then(done)
.catch(done.fail);
@ -78,11 +80,11 @@ describe('AjaxFormVariableList', () => {
it('hides any previous error box', (done) => {
mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200);
expect(errorBox.classList.contains('hide')).toEqual(true);
expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
ajaxVariableList.onSaveClicked()
.then(() => {
expect(errorBox.classList.contains('hide')).toEqual(true);
expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
})
.then(done)
.catch(done.fail);
@ -103,17 +105,39 @@ describe('AjaxFormVariableList', () => {
.catch(done.fail);
});
it('hides secret values', (done) => {
mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200, {});
const row = container.querySelector('.js-row:first-child');
const valueInput = row.querySelector('.js-ci-variable-input-value');
const valuePlaceholder = row.querySelector('.js-secret-value-placeholder');
valueInput.value = 'bar';
$(valueInput).trigger('input');
expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(true);
expect(valueInput.classList.contains(HIDE_CLASS)).toBe(false);
ajaxVariableList.onSaveClicked()
.then(() => {
expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(false);
expect(valueInput.classList.contains(HIDE_CLASS)).toBe(true);
})
.then(done)
.catch(done.fail);
});
it('shows error box with validation errors', (done) => {
const validationError = 'some validation error';
mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(400, [
validationError,
]);
expect(errorBox.classList.contains('hide')).toEqual(true);
expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
ajaxVariableList.onSaveClicked()
.then(() => {
expect(errorBox.classList.contains('hide')).toEqual(false);
expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(false);
expect(errorBox.textContent.trim().replace(/\n+\s+/m, ' ')).toEqual(`Validation failed ${validationError}`);
})
.then(done)
@ -123,11 +147,11 @@ describe('AjaxFormVariableList', () => {
it('shows flash message when request fails', (done) => {
mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(500);
expect(errorBox.classList.contains('hide')).toEqual(true);
expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
ajaxVariableList.onSaveClicked()
.then(() => {
expect(errorBox.classList.contains('hide')).toEqual(true);
expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
})
.then(done)
.catch(done.fail);
@ -170,9 +194,9 @@ describe('AjaxFormVariableList', () => {
const valueInput = row.querySelector('.js-ci-variable-input-value');
keyInput.value = 'foo';
keyInput.dispatchEvent(new Event('input'));
$(keyInput).trigger('input');
valueInput.value = 'bar';
valueInput.dispatchEvent(new Event('input'));
$(valueInput).trigger('input');
expect(idInput.value).toEqual('');

View File

@ -1,6 +1,8 @@
import VariableList from '~/ci_variable_list/ci_variable_list';
import getSetTimeoutPromise from '../helpers/set_timeout_promise_helper';
const HIDE_CLASS = 'hide';
describe('VariableList', () => {
preloadFixtures('pipeline_schedules/edit.html.raw');
preloadFixtures('pipeline_schedules/edit_with_variables.html.raw');
@ -92,14 +94,14 @@ describe('VariableList', () => {
const $inputValue = $row.find('.js-ci-variable-input-value');
const $placeholder = $row.find('.js-secret-value-placeholder');
expect($placeholder.hasClass('hide')).toBe(false);
expect($inputValue.hasClass('hide')).toBe(true);
expect($placeholder.hasClass(HIDE_CLASS)).toBe(false);
expect($inputValue.hasClass(HIDE_CLASS)).toBe(true);
// Reveal values
$wrapper.find('.js-secret-value-reveal-button').click();
expect($placeholder.hasClass('hide')).toBe(true);
expect($inputValue.hasClass('hide')).toBe(false);
expect($placeholder.hasClass(HIDE_CLASS)).toBe(true);
expect($inputValue.hasClass(HIDE_CLASS)).toBe(false);
});
});
});
@ -179,4 +181,35 @@ describe('VariableList', () => {
expect($wrapper.find('.js-ci-variable-input-key:not([disabled])').length).toBe(3);
});
});
describe('hideValues', () => {
beforeEach(() => {
loadFixtures('projects/ci_cd_settings.html.raw');
$wrapper = $('.js-ci-variable-list-section');
variableList = new VariableList({
container: $wrapper,
formField: 'variables',
});
variableList.init();
});
it('should hide value input and show placeholder stars', () => {
const $row = $wrapper.find('.js-row');
const $inputValue = $row.find('.js-ci-variable-input-value');
const $placeholder = $row.find('.js-secret-value-placeholder');
$row.find('.js-ci-variable-input-value')
.val('foo')
.trigger('input');
expect($placeholder.hasClass(HIDE_CLASS)).toBe(true);
expect($inputValue.hasClass(HIDE_CLASS)).toBe(false);
variableList.hideValues();
expect($placeholder.hasClass(HIDE_CLASS)).toBe(false);
expect($inputValue.hasClass(HIDE_CLASS)).toBe(true);
});
});
});