diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/controllers/admin/application_settings_controller.rb | 9 | ||||
| -rw-r--r-- | app/helpers/application_settings_helper.rb | 14 | ||||
| -rw-r--r-- | app/helpers/auth_helper.rb | 10 | ||||
| -rw-r--r-- | app/models/application_setting.rb | 12 | ||||
| -rw-r--r-- | app/views/admin/application_settings/_form.html.haml | 7 | ||||
| -rw-r--r-- | app/views/devise/sessions/new.html.haml | 2 | ||||
| -rw-r--r-- | app/views/devise/shared/_omniauth_box.html.haml | 2 | ||||
| -rw-r--r-- | db/migrate/20160504091942_add_disabled_oauth_sign_in_sources_to_application_settings.rb | 5 | ||||
| -rw-r--r-- | db/schema.rb | 1 | ||||
| -rw-r--r-- | doc/integration/img/enabled-oauth-sign-in-sources.png | bin | 0 -> 49081 bytes | |||
| -rw-r--r-- | doc/integration/omniauth.md | 15 | ||||
| -rw-r--r-- | spec/helpers/auth_helper_spec.rb | 47 | ||||
| -rw-r--r-- | spec/models/application_setting_spec.rb | 9 |
14 files changed, 130 insertions, 4 deletions
diff --git a/CHANGELOG b/CHANGELOG index 777b211daab..efb6dc6f610 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -46,6 +46,7 @@ v 8.8.0 (unreleased) - Fix adding a todo for private group members (Ahmad Sherif) - Bump ace-rails-ap gem version from 2.0.1 to 4.0.2 which upgrades Ace Editor from 1.1.2 to 1.2.3 - Total method execution timings are no longer tracked + - Allow Admins to remove the Login with buttons for OAuth services and still be able to import !4034. (Andrei Gliga) v 8.7.5 - Fix relative links in wiki pages. !4050 diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index ec22548ddeb..b9eb7ae7921 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -53,6 +53,12 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController end end + enabled_oauth_sign_in_sources = params[:application_setting].delete(: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) + params.require(:application_setting).permit( :default_projects_limit, :default_branch_protection, @@ -95,7 +101,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :repository_checks_enabled, :metrics_packet_size, restricted_visibility_levels: [], - import_sources: [] + import_sources: [], + disabled_oauth_sign_in_sources: [] ) end end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 914b0ef6042..03080d25931 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -60,4 +60,18 @@ module ApplicationSettingsHelper end end end + + def oauth_providers_checkboxes + button_based_providers.map do |source| + disabled = current_application_settings.disabled_oauth_sign_in_sources.include?(source.to_s) + css_class = 'btn' + css_class << ' active' unless disabled + checkbox_name = 'application_setting[enabled_oauth_sign_in_sources][]' + + label_tag(checkbox_name, class: css_class) do + check_box_tag(checkbox_name, source, !disabled, + autocomplete: 'off') + Gitlab::OAuth::Provider.label_for(source) + end + end + end end diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index b4f80fd9b3e..b05fa0a14d6 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -38,6 +38,16 @@ module AuthHelper auth_providers.reject { |provider| form_based_provider?(provider) } end + def enabled_button_based_providers + disabled_providers = current_application_settings.disabled_oauth_sign_in_sources || [] + + button_based_providers.map(&:to_s) - disabled_providers + end + + def button_based_providers_enabled? + enabled_button_based_providers.any? + end + def provider_image_tag(provider, size = 64) label = label_for_provider(provider) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 7039db2d41e..c143cf215e6 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -6,6 +6,7 @@ class ApplicationSetting < ActiveRecord::Base serialize :restricted_visibility_levels serialize :import_sources + serialize :disabled_oauth_sign_in_sources serialize :restricted_signup_domains, Array attr_accessor :restricted_signup_domains_raw @@ -69,6 +70,16 @@ class ApplicationSetting < ActiveRecord::Base end end + validates_each :disabled_oauth_sign_in_sources do |record, attr, value| + unless value.nil? + 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 + end + before_save :ensure_runners_registration_token after_commit do @@ -107,6 +118,7 @@ class ApplicationSetting < ActiveRecord::Base recaptcha_enabled: false, akismet_enabled: false, repository_checks_enabled: true, + disabled_oauth_sign_in_sources: [] ) end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index e0d8d16a954..f7c799c968f 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -109,6 +109,13 @@ = f.label :signin_enabled do = f.check_box :signin_enabled Sign-in enabled + - 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' + .col-sm-10 + .btn-group{ data: { toggle: 'buttons' } } + - oauth_providers_checkboxes.each do |source| + = source .form-group = f.label :two_factor_authentication, 'Two-factor authentication', class: 'control-label col-sm-2' .col-sm-10 diff --git a/app/views/devise/sessions/new.html.haml b/app/views/devise/sessions/new.html.haml index d65fa60025c..28194506acc 100644 --- a/app/views/devise/sessions/new.html.haml +++ b/app/views/devise/sessions/new.html.haml @@ -4,7 +4,7 @@ = render 'devise/shared/signin_box' -# Omniauth fits between signin/ldap signin and signup and does not have a surrounding box - - if omniauth_enabled? && devise_mapping.omniauthable? + - if omniauth_enabled? && devise_mapping.omniauthable? && button_based_providers_enabled? .clearfix.prepend-top-20 = render 'devise/shared/omniauth_box' diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml index ecf680e7b23..de18bc2d844 100644 --- a/app/views/devise/shared/_omniauth_box.html.haml +++ b/app/views/devise/shared/_omniauth_box.html.haml @@ -1,7 +1,7 @@ %p %span.light Sign in with - - providers = button_based_providers + - providers = enabled_button_based_providers - providers.each do |provider| %span.light - has_icon = provider_has_icon?(provider) diff --git a/db/migrate/20160504091942_add_disabled_oauth_sign_in_sources_to_application_settings.rb b/db/migrate/20160504091942_add_disabled_oauth_sign_in_sources_to_application_settings.rb new file mode 100644 index 00000000000..facd33875ba --- /dev/null +++ b/db/migrate/20160504091942_add_disabled_oauth_sign_in_sources_to_application_settings.rb @@ -0,0 +1,5 @@ +class AddDisabledOauthSignInSourcesToApplicationSettings < ActiveRecord::Migration + def change + add_column :application_settings, :disabled_oauth_sign_in_sources, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 71d953afe30..74facd12088 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -80,6 +80,7 @@ ActiveRecord::Schema.define(version: 20160508194200) do t.boolean "repository_checks_enabled", default: false t.text "shared_runners_text" t.integer "metrics_packet_size", default: 1 + t.text "disabled_oauth_sign_in_sources" end create_table "audit_events", force: :cascade do |t| diff --git a/doc/integration/img/enabled-oauth-sign-in-sources.png b/doc/integration/img/enabled-oauth-sign-in-sources.png Binary files differnew file mode 100644 index 00000000000..95f8bbdcd24 --- /dev/null +++ b/doc/integration/img/enabled-oauth-sign-in-sources.png diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md index cab329c0dec..820f40f81a9 100644 --- a/doc/integration/omniauth.md +++ b/doc/integration/omniauth.md @@ -11,6 +11,7 @@ of the configured mechanisms. - [Supported Providers](#supported-providers) - [Enable OmniAuth for an Existing User](#enable-omniauth-for-an-existing-user) - [OmniAuth configuration sample when using Omnibus GitLab](https://gitlab.com/gitlab-org/omnibus-gitlab/tree/master#omniauth-google-twitter-github-login) +- [Enable or disable Sign In with an OmniAuth provider without disabling import sources](#enable-or-disable-sign-in-with-an-omniauth-provider-without-disabling-import-sources) ## Supported Providers @@ -191,3 +192,17 @@ experience [in the public Wiki](https://github.com/gitlabhq/gitlab-public-wiki/w While we can't officially support every possible authentication mechanism out there, we'd like to at least help those with specific needs. + +## Enable or disable Sign In with an OmniAuth provider without disabling import sources + +>**Note:** +This setting was introduced with version 8.8 of GitLab. + +Administrators are able to enable or disable Sign In via some OmniAuth providers. + +>**Note:** +By default Sign In is enabled via all the OAuth Providers that have been configured in `config/gitlab.yml`. + +In order to enable/disable an OmniAuth provider, go to Admin Area -> Settings -> Sign-in Restrictions section -> Enabled OAuth Sign-In sources and select the providers you want to enable or disable. + + diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb index e47a54fdac5..16fbb5dcecb 100644 --- a/spec/helpers/auth_helper_spec.rb +++ b/spec/helpers/auth_helper_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe AuthHelper do describe "button_based_providers" do - it 'returns all enabled providers' do + it 'returns all enabled providers from devise' do allow(helper).to receive(:auth_providers) { [:twitter, :github] } expect(helper.button_based_providers).to include(*[:twitter, :github]) end @@ -17,4 +17,49 @@ describe AuthHelper do expect(helper.button_based_providers).to eq([]) end end + + describe 'enabled_button_based_providers' do + before do + allow(helper).to receive(:auth_providers) { [:twitter, :github] } + end + + context 'all providers are enabled to sign in' do + it 'returns all the enabled providers from settings' do + expect(helper.enabled_button_based_providers).to include('twitter', 'github') + end + end + + context 'GitHub OAuth sign in is disabled from application setting' do + it "doesn't return github as provider" do + stub_application_setting( + disabled_oauth_sign_in_sources: ['github'] + ) + + expect(helper.enabled_button_based_providers).to include('twitter') + expect(helper.enabled_button_based_providers).to_not include('github') + end + end + end + + describe 'button_based_providers_enabled?' do + before do + allow(helper).to receive(:auth_providers) { [:twitter, :github] } + end + + context 'button based providers enabled' do + it 'returns true' do + expect(helper.button_based_providers_enabled?).to be true + end + end + + context 'all the button based providers are disabled via application_setting' do + it 'returns false' do + stub_application_setting( + disabled_oauth_sign_in_sources: ['github', 'twitter'] + ) + + expect(helper.button_based_providers_enabled?).to be false + end + end + end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 1ce22feed5c..d84f3e998f5 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -20,6 +20,15 @@ describe ApplicationSetting, models: true do 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) } + 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 + it { is_expected.to validate_presence_of(:max_attachment_size) } it do |
