diff options
26 files changed, 140 insertions, 108 deletions
diff --git a/CHANGELOG b/CHANGELOG index 9371877dcb4..9ffbf64fe1d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -77,6 +77,7 @@ v 7.12.2 - Faster automerge check and merge itself when source and target branches are in same repository - Audit log for user authentication - Fix transferring of project to another group using the API. + - Allow custom label to be set for authentication providers. v 7.12.1 - Fix error when deleting a user who has projects (Stan Hu) @@ -1599,4 +1600,4 @@ v 0.8.0 - stability - security fixes - increased test coverage - - email notification
\ No newline at end of file + - email notification diff --git a/app/assets/images/authbuttons/bitbucket_64.png b/app/assets/images/auth_buttons/bitbucket_64.png Binary files differindex 4b90a57bc7d..4b90a57bc7d 100644 --- a/app/assets/images/authbuttons/bitbucket_64.png +++ b/app/assets/images/auth_buttons/bitbucket_64.png diff --git a/app/assets/images/auth_buttons/github_64.png b/app/assets/images/auth_buttons/github_64.png Binary files differnew file mode 100644 index 00000000000..182a1a3f734 --- /dev/null +++ b/app/assets/images/auth_buttons/github_64.png diff --git a/app/assets/images/auth_buttons/gitlab_64.png b/app/assets/images/auth_buttons/gitlab_64.png Binary files differnew file mode 100644 index 00000000000..99a40583b3a --- /dev/null +++ b/app/assets/images/auth_buttons/gitlab_64.png diff --git a/app/assets/images/authbuttons/google_64.png b/app/assets/images/auth_buttons/google_64.png Binary files differindex fb64f8bee68..fb64f8bee68 100644 --- a/app/assets/images/authbuttons/google_64.png +++ b/app/assets/images/auth_buttons/google_64.png diff --git a/app/assets/images/authbuttons/twitter_64.png b/app/assets/images/auth_buttons/twitter_64.png Binary files differindex e3bd9169a34..e3bd9169a34 100644 --- a/app/assets/images/authbuttons/twitter_64.png +++ b/app/assets/images/auth_buttons/twitter_64.png diff --git a/app/assets/images/authbuttons/github_64.png b/app/assets/images/authbuttons/github_64.png Binary files differdeleted file mode 100644 index dc7c03d1005..00000000000 --- a/app/assets/images/authbuttons/github_64.png +++ /dev/null diff --git a/app/assets/images/authbuttons/gitlab_64.png b/app/assets/images/authbuttons/gitlab_64.png Binary files differdeleted file mode 100644 index 31281a19444..00000000000 --- a/app/assets/images/authbuttons/gitlab_64.png +++ /dev/null diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 362b03e0d5e..3ce8dbc9407 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -299,14 +299,14 @@ class ApplicationController < ActionController::Base end def github_import_enabled? - OauthHelper.enabled_oauth_providers.include?(:github) + Gitlab::OAuth::Provider.enabled?(:github) end def gitlab_import_enabled? - OauthHelper.enabled_oauth_providers.include?(:gitlab) + Gitlab::OAuth::Provider.enabled?(:gitlab) end def bitbucket_import_enabled? - OauthHelper.enabled_oauth_providers.include?(:bitbucket) && Gitlab::BitbucketImport.public_key.present? + Gitlab::OAuth::Provider.enabled?(:bitbucket) && Gitlab::BitbucketImport.public_key.present? end end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index fd51b380da2..523264b8ea9 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -72,10 +72,11 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController end end rescue Gitlab::OAuth::SignupDisabledError => e - message = "Signing in using your #{oauth['provider']} account without a pre-existing GitLab account is not allowed." + label = Gitlab::OAuth::Provider.label_for(oauth['provider']) + message = "Signing in using your #{label} account without a pre-existing GitLab account is not allowed." if current_application_settings.signup_enabled? - message << " Create a GitLab account first, and then connect it to your #{oauth['provider']} account." + message << " Create a GitLab account first, and then connect it to your #{label} account." end flash[:notice] = message diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 89629bc0581..796cbe4c58c 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -90,7 +90,7 @@ class SessionsController < Devise::SessionsController # Prevent alert from popping up on the first page shown after authentication. flash[:alert] = nil - redirect_to omniauth_authorize_path(:user, provider.to_sym) + redirect_to user_omniauth_authorize_path(provider.to_sym) end def valid_otp_attempt?(user) diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb new file mode 100644 index 00000000000..0e7a37b4cc6 --- /dev/null +++ b/app/helpers/auth_helper.rb @@ -0,0 +1,50 @@ +module AuthHelper + PROVIDERS_WITH_ICONS = %w(twitter github gitlab bitbucket google_oauth2).freeze + FORM_BASED_PROVIDERS = [/\Aldap/, 'kerberos'].freeze + + def ldap_enabled? + Gitlab.config.ldap.enabled + end + + def provider_has_icon?(name) + PROVIDERS_WITH_ICONS.include?(name.to_s) + end + + def auth_providers + Gitlab::OAuth::Provider.providers + end + + def label_for_provider(name) + Gitlab::OAuth::Provider.label_for(name) + end + + def form_based_provider?(name) + FORM_BASED_PROVIDERS.any? { |pattern| pattern === name.to_s } + end + + def form_based_providers + auth_providers.select { |provider| form_based_provider?(provider) } + end + + def button_based_providers + auth_providers.reject { |provider| form_based_provider?(provider) } + end + + def provider_image_tag(provider, size = 64) + label = label_for_provider(provider) + + if provider_has_icon?(provider) + file_name = "#{provider.to_s.split('_').first}_#{size}.png" + + image_tag(image_path("auth_buttons/#{file_name}"), alt: label, title: "Sign in with #{label}") + else + label + end + end + + def auth_active?(provider) + current_user.identities.exists?(provider: provider.to_s) + end + + extend self +end diff --git a/app/helpers/oauth_helper.rb b/app/helpers/oauth_helper.rb deleted file mode 100644 index 2fdca13ed40..00000000000 --- a/app/helpers/oauth_helper.rb +++ /dev/null @@ -1,34 +0,0 @@ -module OauthHelper - def ldap_enabled? - Gitlab.config.ldap.enabled - end - - def default_providers - [:twitter, :github, :gitlab, :bitbucket, :google_oauth2, :ldap] - end - - def enabled_oauth_providers - Devise.omniauth_providers - end - - def enabled_social_providers - enabled_oauth_providers.select do |name| - [:saml, :twitter, :gitlab, :github, :bitbucket, :google_oauth2].include?(name.to_sym) - end - end - - def additional_providers - enabled_oauth_providers.reject{|provider| provider.to_s.starts_with?('ldap')} - end - - def oauth_image_tag(provider, size = 64) - file_name = "#{provider.to_s.split('_').first}_#{size}.png" - image_tag(image_path("authbuttons/#{file_name}"), alt: "Sign in with #{provider.to_s.titleize}") - end - - def oauth_active?(provider) - current_user.identities.exists?(provider: provider.to_s) - end - - extend self -end diff --git a/app/helpers/profile_helper.rb b/app/helpers/profile_helper.rb deleted file mode 100644 index 780c7cd5133..00000000000 --- a/app/helpers/profile_helper.rb +++ /dev/null @@ -1,13 +0,0 @@ -module ProfileHelper - def show_profile_username_tab? - current_user.can_change_username? - end - - def show_profile_social_tab? - enabled_social_providers.any? - end - - def show_profile_remove_tab? - signup_enabled? - end -end diff --git a/app/views/admin/identities/_form.html.haml b/app/views/admin/identities/_form.html.haml index 0525552ebf8..3a788558226 100644 --- a/app/views/admin/identities/_form.html.haml +++ b/app/views/admin/identities/_form.html.haml @@ -8,7 +8,8 @@ .form-group = f.label :provider, class: 'control-label' .col-sm-10 - = f.select :provider, Gitlab::OAuth::Provider.names, { allow_blank: false }, class: 'form-control' + - values = Gitlab::OAuth::Provider.providers.map { |name| ["#{Gitlab::OAuth::Provider.label_for(name)} (#{name})", name] } + = f.select :provider, values, { allow_blank: false }, class: 'form-control' .form-group = f.label :extern_uid, "Identifier", class: 'control-label' .col-sm-10 diff --git a/app/views/admin/identities/_identity.html.haml b/app/views/admin/identities/_identity.html.haml index 671c4fbc677..7362d904b94 100644 --- a/app/views/admin/identities/_identity.html.haml +++ b/app/views/admin/identities/_identity.html.haml @@ -1,6 +1,6 @@ %tr %td - = identity.provider + = "#{Gitlab::OAuth::Provider.label_for(identity.provider)} (#{identity.provider})" %td = identity.extern_uid %td diff --git a/app/views/devise/sessions/_new_ldap.html.haml b/app/views/devise/sessions/_new_ldap.html.haml index 6ec741e4882..689cd6ed665 100644 --- a/app/views/devise/sessions/_new_ldap.html.haml +++ b/app/views/devise/sessions/_new_ldap.html.haml @@ -6,4 +6,4 @@ %label{for: "remember_me"} = check_box_tag :remember_me, '1', false, id: 'remember_me' %span Remember me - = button_tag "#{server['label']} Sign in", class: "btn-save btn" + = button_tag "Sign in", class: "btn-save btn" diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml index f8ba9d80ae8..ecf680e7b23 100644 --- a/app/views/devise/shared/_omniauth_box.html.haml +++ b/app/views/devise/shared/_omniauth_box.html.haml @@ -1,10 +1,8 @@ %p %span.light Sign in with - - providers = additional_providers + - providers = button_based_providers - providers.each do |provider| %span.light - - if default_providers.include?(provider) - = link_to oauth_image_tag(provider), omniauth_authorize_path(resource_name, provider), method: :post, class: 'oauth-image-link' - - else - = link_to provider.to_s.titleize, omniauth_authorize_path(resource_name, provider), method: :post, class: "btn", "data-no-turbolink" => "true" + - has_icon = provider_has_icon?(provider) + = link_to provider_image_tag(provider), user_omniauth_authorize_path(provider), method: :post, class: (has_icon ? 'oauth-image-link' : 'btn'), "data-no-turbolink" => "true" diff --git a/app/views/devise/shared/_signin_box.html.haml b/app/views/devise/shared/_signin_box.html.haml index c76574db457..bb5e479697d 100644 --- a/app/views/devise/shared/_signin_box.html.haml +++ b/app/views/devise/shared/_signin_box.html.haml @@ -6,7 +6,7 @@ .login-heading %h3 Sign in .login-body - - if ldap_enabled? + - if form_based_providers.any? %ul.nav.nav-tabs - @ldap_servers.each_with_index do |server, i| %li{class: (:active if i.zero?)} diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 378dfa2dce0..767fe2e0e9a 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -59,22 +59,22 @@ %div = link_to 'Enable Two-factor Authentication', new_profile_two_factor_auth_path, class: 'btn btn-success' - - if show_profile_social_tab? + - if button_based_providers.any? .panel.panel-default .panel-heading Connected Accounts .panel-body .oauth-buttons.append-bottom-10 %p Click on icon to activate signin with one of the following services - - enabled_social_providers.each do |provider| + - button_based_providers.each do |provider| .btn-group - = link_to oauth_image_tag(provider), omniauth_authorize_path(User, provider), - method: :post, class: "btn btn-lg #{'active' if oauth_active?(provider)}" - - if oauth_active?(provider) + = link_to provider_image_tag(provider), user_omniauth_authorize_path(provider), method: :post, class: "btn btn-lg #{'active' if auth_active?(provider)}", "data-no-turbolink" => "true" + + - if auth_active?(provider) = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'btn btn-lg' do = icon('close') - - if show_profile_username_tab? + - if current_user.can_change_username? .panel.panel-warning.update-username .panel-heading Change Username @@ -94,7 +94,7 @@ %div = f.submit 'Save username', class: "btn btn-warning" - - if show_profile_remove_tab? + - if signup_enabled? .panel.panel-danger.remove-account .panel-heading Remove account diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index c32ac2042d0..41a01e7703c 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -209,20 +209,29 @@ production: &base # arguments, followed by optional 'args' which can be either a hash or an array. # Documentation for this is available at http://doc.gitlab.com/ce/integration/omniauth.html providers: - # - { name: 'google_oauth2', app_id: 'YOUR_APP_ID', + # - { name: 'google_oauth2', + # label: 'Google', + # app_id: 'YOUR_APP_ID', # app_secret: 'YOUR_APP_SECRET', # args: { access_type: 'offline', approval_prompt: '' } } - # - { name: 'twitter', app_id: 'YOUR_APP_ID', - # app_secret: 'YOUR_APP_SECRET'} - # - { name: 'github', app_id: 'YOUR_APP_ID', + # - { name: 'twitter', + # app_id: 'YOUR_APP_ID', + # app_secret: 'YOUR_APP_SECRET' } + # - { name: 'github', + # label: 'GitHub', + # app_id: 'YOUR_APP_ID', # app_secret: 'YOUR_APP_SECRET', # args: { scope: 'user:email' } } - # - { name: 'gitlab', app_id: 'YOUR_APP_ID', + # - { name: 'gitlab', + # label: 'GitLab.com', + # app_id: 'YOUR_APP_ID', # app_secret: 'YOUR_APP_SECRET', # args: { scope: 'api' } } - # - { name: 'bitbucket', app_id: 'YOUR_APP_ID', - # app_secret: 'YOUR_APP_SECRET'} - # - { name: 'saml', + # - { name: 'bitbucket', + # app_id: 'YOUR_APP_ID', + # app_secret: 'YOUR_APP_SECRET' } + # - { name: 'saml', + # label: 'Our SAML Provider', # args: { # assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', # idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', diff --git a/doc/integration/omniauth.md b/doc/integration/omniauth.md index 8e2a602ec35..2010cb9b8a1 100644 --- a/doc/integration/omniauth.md +++ b/doc/integration/omniauth.md @@ -84,7 +84,7 @@ Existing users can enable OmniAuth for specific providers after the account is c 1. Sign in normally - whether standard sign in, LDAP, or another OmniAuth provider. 1. Go to profile settings (the silhouette icon in the top right corner). 1. Select the "Account" tab. -1. Under "Social Accounts" select the desired OmniAuth provider, such as Twitter. +1. Under "Connected Accounts" select the desired OmniAuth provider, such as Twitter. 1. The user will be redirected to the provider. Once the user authorized GitLab they will be redirected back to GitLab. The chosen OmniAuth provider is now active and can be used to sign in to GitLab from then on. diff --git a/features/steps/admin/users.rb b/features/steps/admin/users.rb index 6c4b91586d6..49e64eeff71 100644 --- a/features/steps/admin/users.rb +++ b/features/steps/admin/users.rb @@ -3,6 +3,14 @@ class Spinach::Features::AdminUsers < Spinach::FeatureSteps include SharedPaths include SharedAdmin + before do + allow(Devise).to receive(:omniauth_providers).and_return([:twitter, :twitter_updated]) + end + + after do + allow(Devise).to receive(:omniauth_providers).and_call_original + end + step 'I should see all users' do User.all.each do |user| expect(page).to have_content user.name @@ -121,7 +129,6 @@ class Spinach::Features::AdminUsers < Spinach::FeatureSteps end step 'I visit "Pete" identities page in admin' do - allow(Gitlab::OAuth::Provider).to receive(:names).and_return(%w(twitter twitter_updated)) visit admin_user_identities_path(@user) end diff --git a/lib/gitlab/o_auth/provider.rb b/lib/gitlab/o_auth/provider.rb index f986499a27c..90c3fe8da33 100644 --- a/lib/gitlab/o_auth/provider.rb +++ b/lib/gitlab/o_auth/provider.rb @@ -1,18 +1,30 @@ module Gitlab module OAuth class Provider - def self.names - providers = [] + def self.providers + Devise.omniauth_providers + end - Gitlab.config.ldap.servers.values.each do |server| - providers << server['provider_name'] - end + def self.enabled?(name) + providers.include?(name.to_sym) + end - Gitlab.config.omniauth.providers.each do |provider| - providers << provider['name'] + def self.ldap_provider?(name) + name.to_s.start_with?('ldap') + end + + def self.config_for(name) + name = name.to_s + if ldap_provider?(name) + Gitlab::LDAP::Config.new(name).options + else + Gitlab.config.omniauth.providers.find { |provider| provider.name == name } end + end - providers + def self.label_for(name) + config = config_for(name) + (config && config['label']) || name.to_s.titleize end end end diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb new file mode 100644 index 00000000000..e47a54fdac5 --- /dev/null +++ b/spec/helpers/auth_helper_spec.rb @@ -0,0 +1,20 @@ +require "spec_helper" + +describe AuthHelper do + describe "button_based_providers" do + it 'returns all enabled providers' do + allow(helper).to receive(:auth_providers) { [:twitter, :github] } + expect(helper.button_based_providers).to include(*[:twitter, :github]) + end + + it 'does not return ldap provider' do + allow(helper).to receive(:auth_providers) { [:twitter, :ldapmain] } + expect(helper.button_based_providers).to include(:twitter) + end + + it 'returns empty array' do + allow(helper).to receive(:auth_providers) { [] } + expect(helper.button_based_providers).to eq([]) + end + end +end diff --git a/spec/helpers/oauth_helper_spec.rb b/spec/helpers/oauth_helper_spec.rb deleted file mode 100644 index 3ef35f35102..00000000000 --- a/spec/helpers/oauth_helper_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -require "spec_helper" - -describe OauthHelper do - describe "additional_providers" do - it 'returns all enabled providers' do - allow(helper).to receive(:enabled_oauth_providers) { [:twitter, :github] } - expect(helper.additional_providers).to include(*[:twitter, :github]) - end - - it 'does not return ldap provider' do - allow(helper).to receive(:enabled_oauth_providers) { [:twitter, :ldapmain] } - expect(helper.additional_providers).to include(:twitter) - end - - it 'returns empty array' do - allow(helper).to receive(:enabled_oauth_providers) { [] } - expect(helper.additional_providers).to eq([]) - end - end -end |
