diff options
5 files changed, 87 insertions, 2 deletions
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 355662bbb38..709834a2bec 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -191,8 +191,10 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController params[:application_setting][:import_sources]&.delete("") params[:application_setting][:restricted_visibility_levels]&.delete("") - params[:application_setting].delete(:elasticsearch_aws_secret_access_key) if params[:application_setting][:elasticsearch_aws_secret_access_key].blank? params[:application_setting][:required_instance_ci_template] = nil if params[:application_setting][:required_instance_ci_template].blank? + + remove_blank_params_for!(:elasticsearch_aws_secret_access_key, :eks_secret_access_key) + # TODO Remove domain_blacklist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-foss/issues/67204) params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file] params.delete(:domain_blacklist_raw) if params[:domain_blacklist] @@ -261,6 +263,10 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController render action end + def remove_blank_params_for!(*keys) + params[:application_setting].delete_if { |setting, value| setting.to_sym.in?(keys) && value.blank? } + end + # overridden in EE def valid_setting_panels VALID_SETTING_PANELS diff --git a/app/views/admin/application_settings/_eks.html.haml b/app/views/admin/application_settings/_eks.html.haml index b1f7ed76281..d959b4f9b43 100644 --- a/app/views/admin/application_settings/_eks.html.haml +++ b/app/views/admin/application_settings/_eks.html.haml @@ -26,6 +26,6 @@ = f.text_field :eks_access_key_id, class: 'form-control' .form-group = f.label :eks_secret_access_key, 'Secret access key', class: 'label-bold' - = f.password_field :eks_secret_access_key, value: @application_setting.eks_secret_access_key, class: 'form-control' + = f.password_field :eks_secret_access_key, autocomplete: 'off', class: 'form-control' = f.submit 'Save changes', class: "btn btn-success" diff --git a/changelogs/unreleased/security-132-remove-eks-details-from-admin-form.yml b/changelogs/unreleased/security-132-remove-eks-details-from-admin-form.yml new file mode 100644 index 00000000000..ce1c48a6345 --- /dev/null +++ b/changelogs/unreleased/security-132-remove-eks-details-from-admin-form.yml @@ -0,0 +1,5 @@ +--- +title: Hide EKS secret key in admin integrations settings +merge_request: +author: +type: security diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index 33764818f79..fe28e791ade 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -155,6 +155,46 @@ describe Admin::ApplicationSettingsController do end end + describe 'PATCH #integrations' do + before do + stub_feature_flags(instance_level_integrations: false) + sign_in(admin) + end + + describe 'EKS integration' do + let(:application_setting) { ApplicationSetting.current } + let(:settings_params) do + { + eks_integration_enabled: '1', + eks_account_id: '123456789012', + eks_access_key_id: 'dummy access key', + eks_secret_access_key: 'dummy secret key' + } + end + + it 'updates EKS settings' do + patch :integrations, params: { application_setting: settings_params } + + expect(application_setting.eks_integration_enabled).to be_truthy + expect(application_setting.eks_account_id).to eq '123456789012' + expect(application_setting.eks_access_key_id).to eq 'dummy access key' + expect(application_setting.eks_secret_access_key).to eq 'dummy secret key' + end + + context 'secret access key is blank' do + let(:settings_params) { { eks_secret_access_key: '' } } + + it 'does not update the secret key' do + application_setting.update!(eks_secret_access_key: 'dummy secret key') + + patch :integrations, params: { application_setting: settings_params } + + expect(application_setting.reload.eks_secret_access_key).to eq 'dummy secret key' + end + end + end + end + describe 'PUT #reset_registration_token' do before do sign_in(admin) diff --git a/spec/views/admin/application_settings/_eks.html.haml_spec.rb b/spec/views/admin/application_settings/_eks.html.haml_spec.rb new file mode 100644 index 00000000000..52434557d3a --- /dev/null +++ b/spec/views/admin/application_settings/_eks.html.haml_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'admin/application_settings/_eks' do + let_it_be(:admin) { create(:admin) } + let(:page) { Capybara::Node::Simple.new(rendered) } + + before do + assign(:application_setting, application_setting) + allow(view).to receive(:current_user) { admin } + allow(view).to receive(:expanded) { true } + end + + shared_examples 'EKS secret access key input' do + it 'renders an empty password field' do + render + expect(rendered).to have_field('Secret access key', type: 'password') + expect(page.find_field('Secret access key').value).to be_blank + end + end + + context 'when eks_secret_access_key is not set' do + let(:application_setting) { build(:application_setting) } + + include_examples 'EKS secret access key input' + end + + context 'when eks_secret_access_key is set' do + let(:application_setting) { build(:application_setting, eks_secret_access_key: 'eks_secret_access_key') } + + include_examples 'EKS secret access key input' + end +end |