diff options
| -rw-r--r-- | app/controllers/admin/application_settings_controller.rb | 11 | ||||
| -rw-r--r-- | app/helpers/application_settings_helper.rb | 4 | ||||
| -rw-r--r-- | app/views/admin/application_settings/_signin.html.haml | 1 | ||||
| -rw-r--r-- | spec/features/admin/admin_settings_spec.rb | 30 |
4 files changed, 31 insertions, 15 deletions
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 5b1b26a0ed7..8958eab0423 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -58,11 +58,14 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController def application_setting_params params[:application_setting] ||= {} - enabled_oauth_sign_in_sources = params[:application_setting].delete(:enabled_oauth_sign_in_sources) + if params[:application_setting].key?(:enabled_oauth_sign_in_sources) + enabled_oauth_sign_in_sources = params[:application_setting].delete(:enabled_oauth_sign_in_sources) + enabled_oauth_sign_in_sources&.delete("") - params[:application_setting][:disabled_oauth_sign_in_sources] = - AuthHelper.button_based_providers.map(&:to_s) - - Array(enabled_oauth_sign_in_sources) + params[:application_setting][:disabled_oauth_sign_in_sources] = + AuthHelper.button_based_providers.map(&:to_s) - + Array(enabled_oauth_sign_in_sources) + end params[:application_setting][:import_sources]&.delete("") params[:application_setting][:restricted_visibility_levels]&.delete("") diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index b3b080e6dcf..3fbb32c5229 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -74,10 +74,12 @@ module ApplicationSettingsHelper css_class = 'btn' css_class << ' active' unless disabled checkbox_name = 'application_setting[enabled_oauth_sign_in_sources][]' + name = Gitlab::Auth::OAuth::Provider.label_for(source) label_tag(checkbox_name, class: css_class) do check_box_tag(checkbox_name, source, !disabled, - autocomplete: 'off') + Gitlab::Auth::OAuth::Provider.label_for(source) + autocomplete: 'off', + id: name.tr(' ', '_')) + name end end end diff --git a/app/views/admin/application_settings/_signin.html.haml b/app/views/admin/application_settings/_signin.html.haml index 864e64b5fa9..48331c40bca 100644 --- a/app/views/admin/application_settings/_signin.html.haml +++ b/app/views/admin/application_settings/_signin.html.haml @@ -24,6 +24,7 @@ - if omniauth_enabled? && button_based_providers.any? .form-group = f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2' + = hidden_field_tag 'application_setting[enabled_oauth_sign_in_sources][]' .col-sm-10 .btn-group{ data: { toggle: 'buttons' } } - oauth_providers_checkboxes.each do |source| diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 50d0a7abe59..7853d2952ea 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -85,6 +85,26 @@ feature 'Admin updates settings' do expect(page).to have_content "Application settings saved successfully" end + scenario 'Modify oauth providers' 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') + + page.within('.as-signin') do + check "Google" + click_button 'Save changes' + end + + expect(page).to have_content "Application settings saved successfully" + expect(Gitlab::CurrentSettings.disabled_oauth_sign_in_sources).not_to include('google_oauth2') + end + scenario 'Change Help page' do page.within('.as-help-page') do fill_in 'Help page text', with: 'Example text' @@ -234,16 +254,6 @@ feature 'Admin updates settings' do expect(find('#service_push_channel').value).to eq '#test_channel' end - context 'sign-in restrictions', :js do - it 'de-activates oauth sign-in source' do - page.within('.as-signin') do - find('input#application_setting_enabled_oauth_sign_in_sources_[value=gitlab]').send_keys(:return) - - expect(find('.btn', text: 'GitLab.com')).not_to have_css('.active') - end - end - end - scenario 'Change Keys settings' do page.within('.as-visibility-access') do select 'Are forbidden', from: 'RSA SSH keys' |