Commit 975dc69e authored by Clement Ho's avatar Clement Ho

Merge branch '42929-hide-new-variable-values' into 'master'

Hide CI secret variable values on save

Closes #42929

See merge request gitlab-org/gitlab-ce!17044
parents 2203d108 c6b2ff8a
......@@ -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);
......
......@@ -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.
......
---
title: Hide CI secret variable values after saving
merge_request: 17044
author:
type: changed
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('');
......
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);
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment