diff --git a/CHANGELOG b/CHANGELOG index 9371877dcb48d2040c34c9563e8d2ecb97413672..9ffbf64fe1df96d73fd820f757088b21a1c531ae 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 similarity index 100% rename from app/assets/images/authbuttons/bitbucket_64.png rename to 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 new file mode 100644 index 0000000000000000000000000000000000000000..182a1a3f734fc1b7d712c68b04c29bad9460d6cd Binary files /dev/null and b/app/assets/images/auth_buttons/github_64.png differ diff --git a/app/assets/images/auth_buttons/gitlab_64.png b/app/assets/images/auth_buttons/gitlab_64.png new file mode 100644 index 0000000000000000000000000000000000000000..99a40583b3a7a4f96f1715350dfd9429b01976eb Binary files /dev/null and b/app/assets/images/auth_buttons/gitlab_64.png differ diff --git a/app/assets/images/authbuttons/google_64.png b/app/assets/images/auth_buttons/google_64.png similarity index 100% rename from app/assets/images/authbuttons/google_64.png rename to 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 similarity index 100% rename from app/assets/images/authbuttons/twitter_64.png rename to 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 deleted file mode 100644 index dc7c03d10052a596f062207ae0fd8e9cf384b2aa..0000000000000000000000000000000000000000 Binary files a/app/assets/images/authbuttons/github_64.png and /dev/null differ diff --git a/app/assets/images/authbuttons/gitlab_64.png b/app/assets/images/authbuttons/gitlab_64.png deleted file mode 100644 index 31281a194441d4e23bb691de89110c6097021ac9..0000000000000000000000000000000000000000 Binary files a/app/assets/images/authbuttons/gitlab_64.png and /dev/null differ diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 362b03e0d5e244de424577e81dc201cfe3fe8dc0..3ce8dbc9407d45a572ca96572202206b27e326f9 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 fd51b380da21d09bbf87f539e6f0175eba289e74..523264b8ea9170b71701901ccd4ac3fca5d56b35 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 89629bc0581e82d1e194f37118358038ccbc5c5b..796cbe4c58c68221c91525d55e46fe3f73f70f62 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 0000000000000000000000000000000000000000..0e7a37b4cc64ce440b245e4f442e32a5ec31e5f5 --- /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 2fdca13ed4086b2641b4ae7300668d3f76ca3d79..0000000000000000000000000000000000000000 --- 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 780c7cd51332193bb526402232fd36ad26d6cb7c..0000000000000000000000000000000000000000 --- 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 0525552ebf86754fe31526a8152ba15c6273383c..3a7885582262b02dd5bb9414952dbb0c1af2aae1 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 671c4fbc6774ab85500d231889fb441a74b6c4bf..7362d904b94dd34c28432635d1189a086e41696e 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 6ec741e48823f70a83040995db1dc36d8a267ce9..689cd6ed6654a0793a0f2b6b5c752a9418a89733 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 f8ba9d80ae86183b35bc518564faff4b50b98d25..ecf680e7b233133275ddc9036497431a3144b4b2 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 c76574db457d3581a3c73a9599ce902e1572aeee..bb5e479697ddb76af49c2412ec64926d3013cf93 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 378dfa2dce08006eb645815ad5c8077c7779a721..767fe2e0e9a9174b66ab96a6921b5b39cfef56a5 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 c32ac2042d0d3dffa45ba8079891b441b6bc2ddd..41a01e7703c7ac7ccc7bf2574a098aeb2e2a9ecf 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 8e2a602ec3500cf8e7d528870d3eef78e2a6ddbe..2010cb9b8a1b710e0a61c33483061c3153494913 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 6c4b91586d65720e1e91a6df8feae00380835788..49e64eeff71c2a13ccec4625f4fa2fcd343b0a46 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 f986499a27cc62e61151da0f8bd6c2b2a72b404f..90c3fe8da3339d10139e8a5a88eaacc633cf8eaf 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 0000000000000000000000000000000000000000..e47a54fdac5e27a92ccfc0b6097062e6759b1262 --- /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 3ef35f35102e310d9867f99674c092d395142694..0000000000000000000000000000000000000000 --- 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