From cc18c332e0b99f7b674a56a5be59e8b118c81289 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Thu, 22 Aug 2019 19:05:43 +0200 Subject: [PATCH] Add test, reduce complexity --- app/services/application_settings/update_service.rb | 7 +++++-- spec/services/application_settings/update_service_spec.rb | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb index 13718734844..8115585b7a8 100644 --- a/app/services/application_settings/update_service.rb +++ b/app/services/application_settings/update_service.rb @@ -7,8 +7,7 @@ module ApplicationSettings attr_reader :params, :application_setting def execute - disable_ext_auth = params[:external_authorization_service_enabled].present? && !Gitlab::Utils.to_boolean(params[:external_authorization_service_enabled]) - validate_classification_label(application_setting, :external_authorization_service_default_label) unless disable_ext_auth + validate_classification_label(application_setting, :external_authorization_service_default_label) unless bypass_external_auth? if application_setting.errors.any? return false @@ -60,5 +59,9 @@ module ApplicationSettings Group.find_by_full_path(group_full_path)&.id if group_full_path.present? end + + def bypass_external_auth? + params.key?(:external_authorization_service_enabled) && !Gitlab::Utils.to_boolean(params[:external_authorization_service_enabled]) + end end end diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb index adb5219d691..235fce92fc8 100644 --- a/spec/services/application_settings/update_service_spec.rb +++ b/spec/services/application_settings/update_service_spec.rb @@ -201,6 +201,12 @@ describe ApplicationSettings::UpdateService do enable_external_authorization_service_check end + it 'does not validate labels if external authorization gets disabled' do + expect_any_instance_of(described_class).not_to receive(:validate_classification_label) + + described_class.new(application_settings, admin, { external_authorization_service_enabled: false }).execute + end + it 'does not save the settings with an error if the service denies access' do expect(::Gitlab::ExternalAuthorization) .to receive(:access_allowed?).with(admin, 'new-label') { false }