Merge branch 'thomas-nilsson-irfu/gitlab-ce-thomas-nilsson-irfu-master-patch-13137' into 'master'

Allow masking if 8 or more characters in base64

See merge request gitlab-org/gitlab-ce!29143
This commit is contained in:
Stan Hu 2019-06-07 13:15:27 +00:00
commit 25ef3a9687
11 changed files with 95 additions and 8 deletions

View file

@ -27,15 +27,24 @@ function generateErrorBoxContent(errors) {
// Used for the variable list on CI/CD projects/groups settings page
export default class AjaxVariableList {
constructor({ container, saveButton, errorBox, formField = 'variables', saveEndpoint }) {
constructor({
container,
saveButton,
errorBox,
formField = 'variables',
saveEndpoint,
maskableRegex,
}) {
this.container = container;
this.saveButton = saveButton;
this.errorBox = errorBox;
this.saveEndpoint = saveEndpoint;
this.maskableRegex = maskableRegex;
this.variableList = new VariableList({
container: this.container,
formField,
maskableRegex,
});
this.bindEvents();

View file

@ -16,9 +16,10 @@ function createEnvironmentItem(value) {
}
export default class VariableList {
constructor({ container, formField }) {
constructor({ container, formField, maskableRegex }) {
this.$container = $(container);
this.formField = formField;
this.maskableRegex = new RegExp(maskableRegex);
this.environmentDropdownMap = new WeakMap();
this.inputMap = {
@ -196,9 +197,8 @@ export default class VariableList {
validateMaskability($row) {
const invalidInputClass = 'gl-field-error-outline';
const maskableRegex = /^\w{8,}$/; // Eight or more alphanumeric characters plus underscores
const variableValue = $row.find(this.inputMap.secret_value.selector).val();
const isValueMaskable = maskableRegex.test(variableValue) || variableValue === '';
const isValueMaskable = this.maskableRegex.test(variableValue) || variableValue === '';
const isMaskedChecked = $row.find(this.inputMap.masked.selector).val() === 'true';
// Show a validation error if the user wants to mask an unmaskable variable value

View file

@ -12,5 +12,6 @@ document.addEventListener('DOMContentLoaded', () => {
saveButton: variableListEl.querySelector('.js-ci-variables-save-button'),
errorBox: variableListEl.querySelector('.js-ci-variable-error-box'),
saveEndpoint: variableListEl.dataset.saveEndpoint,
maskableRegex: variableListEl.dataset.maskableRegex,
});
});

View file

@ -21,6 +21,7 @@ document.addEventListener('DOMContentLoaded', () => {
saveButton: variableListEl.querySelector('.js-ci-variables-save-button'),
errorBox: variableListEl.querySelector('.js-ci-variable-error-box'),
saveEndpoint: variableListEl.dataset.saveEndpoint,
maskableRegex: variableListEl.dataset.maskableRegex,
});
// hide extra auto devops settings based checkbox state

View file

@ -27,4 +27,8 @@ module CiVariablesHelper
%w(File file)
]
end
def ci_variable_maskable_regex
Maskable::REGEX.inspect.sub('\\A', '^').sub('\\z', '$').sub(/^\//, '').sub(/\/[a-z]*$/, '').gsub('\/', '/')
end
end

View file

@ -7,9 +7,9 @@ module Maskable
# * No escape characters
# * No variables
# * No spaces
# * Minimal length of 8 characters
# * Minimal length of 8 characters from the Base64 alphabets (RFC4648)
# * Absolutely no fun is allowed
REGEX = /\A\w{8,}\z/.freeze
REGEX = /\A[a-zA-Z0-9_+=\/-]{8,}\z/.freeze
included do
validates :masked, inclusion: { in: [true, false] }

View file

@ -6,7 +6,7 @@
= s_('Environment variables are configured by your administrator to be %{link_start}protected%{link_end} by default').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
.row
.col-lg-12.js-ci-variable-list-section{ data: { save_endpoint: save_endpoint } }
.col-lg-12.js-ci-variable-list-section{ data: { save_endpoint: save_endpoint, maskable_regex: ci_variable_maskable_regex } }
.hide.alert.alert-danger.js-ci-variable-error-box
%ul.ci-variable-list

View file

@ -0,0 +1,5 @@
---
title: Allow masking if 8 or more characters in base64.
merge_request: 29143
author: thomas-nilsson-irfu
type: changed

View file

@ -92,7 +92,7 @@ This means that the value of the variable will be hidden in job logs,
though it must match certain requirements to do so:
- The value must be in a single line.
- The value must contain only letters, numbers, or underscores.
- The value must only consist of characters from the Base64 alphabet, defined in [RFC4648](https://tools.ietf.org/html/rfc4648).
- The value must be at least 8 characters long.
- The value must not use variables.

View file

@ -32,6 +32,7 @@ describe('AjaxFormVariableList', () => {
saveButton,
errorBox,
saveEndpoint: container.dataset.saveEndpoint,
maskableRegex: container.dataset.maskableRegex,
});
spyOn(ajaxVariableList, 'updateRowsWithPersistedVariables').and.callThrough();
@ -220,4 +221,11 @@ describe('AjaxFormVariableList', () => {
expect(row.dataset.isPersisted).toEqual('true');
});
});
describe('maskableRegex', () => {
it('takes in the regex provided by the data attribute', () => {
expect(container.dataset.maskableRegex).toBe('^[a-zA-Z0-9_+=/-]{8,}$');
expect(ajaxVariableList.maskableRegex).toBe(container.dataset.maskableRegex);
});
});
});

View file

@ -150,6 +150,65 @@ describe('VariableList', () => {
.then(done)
.catch(done.fail);
});
describe('validateMaskability', () => {
let $row;
const maskingErrorElement = '.js-row:last-child .masking-validation-error';
beforeEach(() => {
$row = $wrapper.find('.js-row:last-child');
$row.find('.ci-variable-masked-item .js-project-feature-toggle').click();
});
it('has a regex provided via a data attribute', () => {
expect($wrapper.attr('data-maskable-regex')).toBe('^[a-zA-Z0-9_+=/-]{8,}$');
});
it('allows values that are 8 characters long', done => {
$row.find('.js-ci-variable-input-value').val('looooong');
getSetTimeoutPromise()
.then(() => {
expect($wrapper.find(maskingErrorElement)).toHaveClass('hide');
})
.then(done)
.catch(done.fail);
});
it('rejects values that are shorter than 8 characters', done => {
$row.find('.js-ci-variable-input-value').val('short');
getSetTimeoutPromise()
.then(() => {
expect($wrapper.find(maskingErrorElement)).toBeVisible();
})
.then(done)
.catch(done.fail);
});
it('allows values with base 64 characters', done => {
$row.find('.js-ci-variable-input-value').val('abcABC123_+=/-');
getSetTimeoutPromise()
.then(() => {
expect($wrapper.find(maskingErrorElement)).toHaveClass('hide');
})
.then(done)
.catch(done.fail);
});
it('rejects values with other special characters', done => {
$row.find('.js-ci-variable-input-value').val('1234567$');
getSetTimeoutPromise()
.then(() => {
expect($wrapper.find(maskingErrorElement)).toBeVisible();
})
.then(done)
.catch(done.fail);
});
});
});
describe('toggleEnableRow method', () => {