Merge branch '66443-unrecoverable-configuration-loop-in-external-auth-control' into 'master'

Resolve "Unrecoverable configuration loop in external auth control"

Closes #66443

See merge request gitlab-org/gitlab-ce!32102
This commit is contained in:
Nick Thomas 2019-08-26 10:22:34 +00:00
commit 74904116d8
3 changed files with 28 additions and 1 deletions

View file

@ -7,7 +7,7 @@ module ApplicationSettings
attr_reader :params, :application_setting attr_reader :params, :application_setting
def execute def execute
validate_classification_label(application_setting, :external_authorization_service_default_label) validate_classification_label(application_setting, :external_authorization_service_default_label) unless bypass_external_auth?
if application_setting.errors.any? if application_setting.errors.any?
return false return false
@ -59,5 +59,9 @@ module ApplicationSettings
Group.find_by_full_path(group_full_path)&.id if group_full_path.present? Group.find_by_full_path(group_full_path)&.id if group_full_path.present?
end end
def bypass_external_auth?
params.key?(:external_authorization_service_enabled) && !Gitlab::Utils.to_boolean(params[:external_authorization_service_enabled])
end
end end
end end

View file

@ -0,0 +1,5 @@
---
title: Don't check external authorization when disabling the service
merge_request: 32102
author: Robert Schilling
type: fixed

View file

@ -201,6 +201,24 @@ describe ApplicationSettings::UpdateService do
enable_external_authorization_service_check enable_external_authorization_service_check
end 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 validate labels if external authorization gets enabled ' do
expect_any_instance_of(described_class).to receive(:validate_classification_label)
described_class.new(application_settings, admin, { external_authorization_service_enabled: true }).execute
end
it 'does validate labels if external authorization is left unchanged' do
expect_any_instance_of(described_class).to receive(:validate_classification_label)
described_class.new(application_settings, admin, { external_authorization_service_default_label: 'new-label' }).execute
end
it 'does not save the settings with an error if the service denies access' do it 'does not save the settings with an error if the service denies access' do
expect(::Gitlab::ExternalAuthorization) expect(::Gitlab::ExternalAuthorization)
.to receive(:access_allowed?).with(admin, 'new-label') { false } .to receive(:access_allowed?).with(admin, 'new-label') { false }