Merge branch '46783-removed-omniauth-provider-causing-invalid-application-setting' into 'master'
Resolve "Removed omniauth provider causing invalid application setting" Closes #46783 See merge request gitlab-org/gitlab-ce!20129
This commit is contained in:
commit
9796ca3689
|
@ -212,14 +212,6 @@ class ApplicationSetting < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
validates_each :disabled_oauth_sign_in_sources do |record, attr, value|
|
|
||||||
value&.each do |source|
|
|
||||||
unless Devise.omniauth_providers.include?(source.to_sym)
|
|
||||||
record.errors.add(attr, "'#{source}' is not an OAuth sign-in source")
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
validate :terms_exist, if: :enforce_terms?
|
validate :terms_exist, if: :enforce_terms?
|
||||||
|
|
||||||
before_validation :ensure_uuid!
|
before_validation :ensure_uuid!
|
||||||
|
@ -330,6 +322,11 @@ class ApplicationSetting < ActiveRecord::Base
|
||||||
::Gitlab::Database.cached_column_exists?(:application_settings, :sidekiq_throttling_enabled)
|
::Gitlab::Database.cached_column_exists?(:application_settings, :sidekiq_throttling_enabled)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def disabled_oauth_sign_in_sources=(sources)
|
||||||
|
sources = (sources || []).map(&:to_s) & Devise.omniauth_providers.map(&:to_s)
|
||||||
|
super(sources)
|
||||||
|
end
|
||||||
|
|
||||||
def domain_whitelist_raw
|
def domain_whitelist_raw
|
||||||
self.domain_whitelist&.join("\n")
|
self.domain_whitelist&.join("\n")
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Ignore unknown OAuth sources in ApplicationSetting
|
||||||
|
merge_request: 20129
|
||||||
|
author:
|
||||||
|
type: fixed
|
|
@ -124,6 +124,29 @@ feature 'Admin updates settings' do
|
||||||
expect(Gitlab::CurrentSettings.disabled_oauth_sign_in_sources).not_to include('google_oauth2')
|
expect(Gitlab::CurrentSettings.disabled_oauth_sign_in_sources).not_to include('google_oauth2')
|
||||||
end
|
end
|
||||||
|
|
||||||
|
scenario 'Oauth providers do not raise validation errors when saving unrelated changes' do
|
||||||
|
expect(Gitlab::CurrentSettings.disabled_oauth_sign_in_sources).to be_empty
|
||||||
|
|
||||||
|
page.within('.as-signin') do
|
||||||
|
uncheck 'Google'
|
||||||
|
click_button 'Save changes'
|
||||||
|
end
|
||||||
|
|
||||||
|
expect(page).to have_content "Application settings saved successfully"
|
||||||
|
expect(Gitlab::CurrentSettings.disabled_oauth_sign_in_sources).to include('google_oauth2')
|
||||||
|
|
||||||
|
# Remove google_oauth2 from the Omniauth strategies
|
||||||
|
allow(Devise).to receive(:omniauth_providers).and_return([])
|
||||||
|
|
||||||
|
# Save an unrelated setting
|
||||||
|
page.within('.as-ci-cd') do
|
||||||
|
click_button 'Save changes'
|
||||||
|
end
|
||||||
|
|
||||||
|
expect(page).to have_content "Application settings saved successfully"
|
||||||
|
expect(Gitlab::CurrentSettings.disabled_oauth_sign_in_sources).to include('google_oauth2')
|
||||||
|
end
|
||||||
|
|
||||||
scenario 'Change Help page' do
|
scenario 'Change Help page' do
|
||||||
page.within('.as-help-page') do
|
page.within('.as-help-page') do
|
||||||
fill_in 'Help page text', with: 'Example text'
|
fill_in 'Help page text', with: 'Example text'
|
||||||
|
|
|
@ -25,15 +25,6 @@ describe ApplicationSetting do
|
||||||
it { is_expected.to allow_value(https).for(:after_sign_out_path) }
|
it { is_expected.to allow_value(https).for(:after_sign_out_path) }
|
||||||
it { is_expected.not_to allow_value(ftp).for(:after_sign_out_path) }
|
it { is_expected.not_to allow_value(ftp).for(:after_sign_out_path) }
|
||||||
|
|
||||||
describe 'disabled_oauth_sign_in_sources validations' do
|
|
||||||
before do
|
|
||||||
allow(Devise).to receive(:omniauth_providers).and_return([:github])
|
|
||||||
end
|
|
||||||
|
|
||||||
it { is_expected.to allow_value(['github']).for(:disabled_oauth_sign_in_sources) }
|
|
||||||
it { is_expected.not_to allow_value(['test']).for(:disabled_oauth_sign_in_sources) }
|
|
||||||
end
|
|
||||||
|
|
||||||
describe 'default_artifacts_expire_in' do
|
describe 'default_artifacts_expire_in' do
|
||||||
it 'sets an error if it cannot parse' do
|
it 'sets an error if it cannot parse' do
|
||||||
setting.update(default_artifacts_expire_in: 'a')
|
setting.update(default_artifacts_expire_in: 'a')
|
||||||
|
@ -314,6 +305,33 @@ describe ApplicationSetting do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#disabled_oauth_sign_in_sources=' do
|
||||||
|
before do
|
||||||
|
allow(Devise).to receive(:omniauth_providers).and_return([:github])
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'removes unknown sources (as strings) from the array' do
|
||||||
|
subject.disabled_oauth_sign_in_sources = %w[github test]
|
||||||
|
|
||||||
|
expect(subject).to be_valid
|
||||||
|
expect(subject.disabled_oauth_sign_in_sources).to eq ['github']
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'removes unknown sources (as symbols) from the array' do
|
||||||
|
subject.disabled_oauth_sign_in_sources = %i[github test]
|
||||||
|
|
||||||
|
expect(subject).to be_valid
|
||||||
|
expect(subject.disabled_oauth_sign_in_sources).to eq ['github']
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'ignores nil' do
|
||||||
|
subject.disabled_oauth_sign_in_sources = nil
|
||||||
|
|
||||||
|
expect(subject).to be_valid
|
||||||
|
expect(subject.disabled_oauth_sign_in_sources).to be_empty
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context 'restricted signup domains' do
|
context 'restricted signup domains' do
|
||||||
it 'sets single domain' do
|
it 'sets single domain' do
|
||||||
setting.domain_whitelist_raw = 'example.com'
|
setting.domain_whitelist_raw = 'example.com'
|
||||||
|
|
Loading…
Reference in New Issue