Allow custom label to be set for authentication providers.auth-icons-labels

26 files changed, 139 insertions, 107 deletions

diff --git a/CHANGELOG b/CHANGELOG
index c6c5bc0aac4..b31ef352549 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -72,6 +72,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)
diff --git a/app/assets/images/authbuttons/bitbucket_64.png b/app/assets/images/auth_buttons/bitbucket_64.png
index 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
new 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
new 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
index 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
index 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
deleted 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
deleted 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 &nbsp;
-  - 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