diff options
13 files changed, 35 insertions, 22 deletions
diff --git a/app/assets/javascripts/ci_variable_list/ci_variable_list.js b/app/assets/javascripts/ci_variable_list/ci_variable_list.js index 745f3404295..e177a3bfdc7 100644 --- a/app/assets/javascripts/ci_variable_list/ci_variable_list.js +++ b/app/assets/javascripts/ci_variable_list/ci_variable_list.js @@ -33,7 +33,7 @@ export default class VariableList { selector: '.js-ci-variable-input-key', default: '', }, - value: { + secret_value: { selector: '.js-ci-variable-input-value', default: '', }, @@ -105,7 +105,7 @@ export default class VariableList { setupToggleButtons($row[0]); // Reset the resizable textarea - $row.find(this.inputMap.value.selector).css('height', ''); + $row.find(this.inputMap.secret_value.selector).css('height', ''); const $environmentSelect = $row.find('.js-variable-environment-toggle'); if ($environmentSelect.length) { diff --git a/app/controllers/groups/variables_controller.rb b/app/controllers/groups/variables_controller.rb index cb8771bc97e..6142e75b4c1 100644 --- a/app/controllers/groups/variables_controller.rb +++ b/app/controllers/groups/variables_controller.rb @@ -39,7 +39,7 @@ module Groups end def variable_params_attributes - %i[id key value protected _destroy] + %i[id key secret_value protected _destroy] end def authorize_admin_build! diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index b478e7b5e05..fa258f3d9af 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -92,7 +92,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController def schedule_params params.require(:schedule) .permit(:description, :cron, :cron_timezone, :ref, :active, - variables_attributes: [:id, :key, :value, :_destroy] ) + variables_attributes: [:id, :key, :secret_value, :_destroy] ) end def authorize_play_pipeline_schedule! diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index 7eb509e2e64..517d0b026c2 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -36,6 +36,6 @@ class Projects::VariablesController < Projects::ApplicationController end def variable_params_attributes - %i[id key value protected _destroy] + %i[id key secret_value protected _destroy] end end diff --git a/app/models/ci/group_variable.rb b/app/models/ci/group_variable.rb index 1dd0e050ba9..62d768cc6cf 100644 --- a/app/models/ci/group_variable.rb +++ b/app/models/ci/group_variable.rb @@ -6,6 +6,8 @@ module Ci belongs_to :group + alias_attribute :secret_value, :value + validates :key, uniqueness: { scope: :group_id, message: "(%{value}) has already been taken" diff --git a/app/models/ci/pipeline_schedule_variable.rb b/app/models/ci/pipeline_schedule_variable.rb index af989fb14b4..03df4e3e638 100644 --- a/app/models/ci/pipeline_schedule_variable.rb +++ b/app/models/ci/pipeline_schedule_variable.rb @@ -5,6 +5,8 @@ module Ci belongs_to :pipeline_schedule + alias_attribute :secret_value, :value + validates :key, uniqueness: { scope: :pipeline_schedule_id } end end diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb index 7c71291de84..452cb910bca 100644 --- a/app/models/ci/variable.rb +++ b/app/models/ci/variable.rb @@ -6,6 +6,8 @@ module Ci belongs_to :project + alias_attribute :secret_value, :value + validates :key, uniqueness: { scope: [:project_id, :environment_scope], message: "(%{value}) has already been taken" diff --git a/app/views/ci/variables/_variable_row.html.haml b/app/views/ci/variables/_variable_row.html.haml index 15201780451..5d4229c80af 100644 --- a/app/views/ci/variables/_variable_row.html.haml +++ b/app/views/ci/variables/_variable_row.html.haml @@ -10,7 +10,7 @@ - id_input_name = "#{form_field}[variables_attributes][][id]" - destroy_input_name = "#{form_field}[variables_attributes][][_destroy]" - key_input_name = "#{form_field}[variables_attributes][][key]" -- value_input_name = "#{form_field}[variables_attributes][][value]" +- value_input_name = "#{form_field}[variables_attributes][][secret_value]" - protected_input_name = "#{form_field}[variables_attributes][][protected]" %li.js-row.ci-variable-row{ data: { is_persisted: "#{!id.nil?}" } } diff --git a/changelogs/unreleased/43316-controller-parameters-handling-sensitive-information-should-use-a-more-specific-name.yml b/changelogs/unreleased/43316-controller-parameters-handling-sensitive-information-should-use-a-more-specific-name.yml new file mode 100644 index 00000000000..de1cee6e436 --- /dev/null +++ b/changelogs/unreleased/43316-controller-parameters-handling-sensitive-information-should-use-a-more-specific-name.yml @@ -0,0 +1,5 @@ +--- +title: Use specific names for filtered CI variable controller parameters +merge_request: 17796 +author: +type: other diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index 966ffdf6996..3506305f755 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -80,7 +80,7 @@ describe Projects::PipelineSchedulesController do context 'when variables_attributes has one variable' do let(:schedule) do basic_param.merge({ - variables_attributes: [{ key: 'AAA', value: 'AAA123' }] + variables_attributes: [{ key: 'AAA', secret_value: 'AAA123' }] }) end @@ -101,7 +101,8 @@ describe Projects::PipelineSchedulesController do context 'when variables_attributes has two variables and duplicated' do let(:schedule) do basic_param.merge({ - variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }] + variables_attributes: [{ key: 'AAA', secret_value: 'AAA123' }, + { key: 'AAA', secret_value: 'BBB123' }] }) end @@ -152,7 +153,7 @@ describe Projects::PipelineSchedulesController do context 'when params include one variable' do let(:schedule) do basic_param.merge({ - variables_attributes: [{ key: 'AAA', value: 'AAA123' }] + variables_attributes: [{ key: 'AAA', secret_value: 'AAA123' }] }) end @@ -169,7 +170,8 @@ describe Projects::PipelineSchedulesController do context 'when params include two duplicated variables' do let(:schedule) do basic_param.merge({ - variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }] + variables_attributes: [{ key: 'AAA', secret_value: 'AAA123' }, + { key: 'AAA', secret_value: 'BBB123' }] }) end @@ -194,7 +196,7 @@ describe Projects::PipelineSchedulesController do context 'when adds a new variable' do let(:schedule) do basic_param.merge({ - variables_attributes: [{ key: 'AAA', value: 'AAA123' }] + variables_attributes: [{ key: 'AAA', secret_value: 'AAA123' }] }) end @@ -209,7 +211,7 @@ describe Projects::PipelineSchedulesController do context 'when adds a new duplicated variable' do let(:schedule) do basic_param.merge({ - variables_attributes: [{ key: 'CCC', value: 'AAA123' }] + variables_attributes: [{ key: 'CCC', secret_value: 'AAA123' }] }) end @@ -224,7 +226,7 @@ describe Projects::PipelineSchedulesController do context 'when updates a variable' do let(:schedule) do basic_param.merge({ - variables_attributes: [{ id: pipeline_schedule_variable.id, value: 'new_value' }] + variables_attributes: [{ id: pipeline_schedule_variable.id, secret_value: 'new_value' }] }) end @@ -252,7 +254,7 @@ describe Projects::PipelineSchedulesController do let(:schedule) do basic_param.merge({ variables_attributes: [{ id: pipeline_schedule_variable.id, _destroy: true }, - { key: 'CCC', value: 'CCC123' }] + { key: 'CCC', secret_value: 'CCC123' }] }) end diff --git a/spec/features/projects/pipeline_schedules_spec.rb b/spec/features/projects/pipeline_schedules_spec.rb index 65e24862d43..065d00d51d4 100644 --- a/spec/features/projects/pipeline_schedules_spec.rb +++ b/spec/features/projects/pipeline_schedules_spec.rb @@ -160,9 +160,9 @@ feature 'Pipeline Schedules', :js do click_link 'New schedule' fill_in_schedule_form all('[name="schedule[variables_attributes][][key]"]')[0].set('AAA') - all('[name="schedule[variables_attributes][][value]"]')[0].set('AAA123') + all('[name="schedule[variables_attributes][][secret_value]"]')[0].set('AAA123') all('[name="schedule[variables_attributes][][key]"]')[1].set('BBB') - all('[name="schedule[variables_attributes][][value]"]')[1].set('BBB123') + all('[name="schedule[variables_attributes][][secret_value]"]')[1].set('BBB123') save_pipeline_schedule end diff --git a/spec/javascripts/ci_variable_list/native_form_variable_list_spec.js b/spec/javascripts/ci_variable_list/native_form_variable_list_spec.js index 1ea8d86cb7e..94a0c999d66 100644 --- a/spec/javascripts/ci_variable_list/native_form_variable_list_spec.js +++ b/spec/javascripts/ci_variable_list/native_form_variable_list_spec.js @@ -20,7 +20,7 @@ describe('NativeFormVariableList', () => { it('should clear out the `name` attribute on the inputs for the last empty row on form submission (avoid BE validation)', () => { const $row = $wrapper.find('.js-row'); expect($row.find('.js-ci-variable-input-key').attr('name')).toBe('schedule[variables_attributes][][key]'); - expect($row.find('.js-ci-variable-input-value').attr('name')).toBe('schedule[variables_attributes][][value]'); + expect($row.find('.js-ci-variable-input-value').attr('name')).toBe('schedule[variables_attributes][][secret_value]'); $wrapper.closest('form').trigger('trigger-submit'); diff --git a/spec/support/shared_examples/controllers/variables_shared_examples.rb b/spec/support/shared_examples/controllers/variables_shared_examples.rb index d7acf8c0032..b615a8f54cf 100644 --- a/spec/support/shared_examples/controllers/variables_shared_examples.rb +++ b/spec/support/shared_examples/controllers/variables_shared_examples.rb @@ -16,19 +16,19 @@ shared_examples 'PATCH #update updates variables' do let(:variable_attributes) do { id: variable.id, key: variable.key, - value: variable.value, + secret_value: variable.value, protected: variable.protected?.to_s } end let(:new_variable_attributes) do { key: 'new_key', - value: 'dummy_value', + secret_value: 'dummy_value', protected: 'false' } end context 'with invalid new variable parameters' do let(:variables_attributes) do [ - variable_attributes.merge(value: 'other_value'), + variable_attributes.merge(secret_value: 'other_value'), new_variable_attributes.merge(key: '...?') ] end @@ -52,7 +52,7 @@ shared_examples 'PATCH #update updates variables' do let(:variables_attributes) do [ new_variable_attributes, - new_variable_attributes.merge(value: 'other_value') + new_variable_attributes.merge(secret_value: 'other_value') ] end @@ -74,7 +74,7 @@ shared_examples 'PATCH #update updates variables' do context 'with valid new variable parameters' do let(:variables_attributes) do [ - variable_attributes.merge(value: 'other_value'), + variable_attributes.merge(secret_value: 'other_value'), new_variable_attributes ] end |