Merge branch 'auth-icons-labels' into 'master'

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

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

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

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)

app/helpers/auth_helper.rb

+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

app/helpers/oauth_helper.rb

-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

app/helpers/profile_helper.rb

-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

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

app/views/admin/identities/_identity.html.haml

 %tr
   %td
-    = identity.provider
+    = "#{Gitlab::OAuth::Provider.label_for(identity.provider)} (#{identity.provider})"
   %td
     = identity.extern_uid
   %td

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"

app/views/devise/shared/_omniauth_box.html.haml

 %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"

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?)}

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

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',

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.

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
 

lib/gitlab/o_auth/provider.rb

 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

spec/helpers/oauth_helper_spec.rb → spec/helpers/auth_helper_spec.rb

 require "spec_helper"
 
-describe OauthHelper do
-  describe "additional_providers" do
+describe AuthHelper do
+  describe "button_based_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])
+      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(:enabled_oauth_providers) { [:twitter, :ldapmain] }
-      expect(helper.additional_providers).to include(:twitter)
+      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(:enabled_oauth_providers) { [] }
-      expect(helper.additional_providers).to eq([])
+      allow(helper).to receive(:auth_providers) { [] }
+      expect(helper.button_based_providers).to eq([])
     end
   end
 end