Backport of ee/8120: Smartcard authenticationif-ee-726-smartcard_auth-ce_port

8 files changed, 72 insertions, 30 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 7f4aa8244ac..b839da7770d 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -181,11 +181,11 @@ class ApplicationController < ActionController::Base
     Ability.allowed?(object, action, subject)
   end
 
-  def access_denied!(message = nil)
+  def access_denied!(message = nil, status = nil)
     # If we display a custom access denied message to the user, we don't want to
     # hide existence of the resource, rather tell them they cannot access it using
     # the provided message
-    status = message.present? ? :forbidden : :not_found
+    status ||= message.present? ? :forbidden : :not_found
 
     respond_to do |format|
       format.any { head status }
diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb
index c158cf20dd6..44f85e9c0f8 100644
--- a/app/helpers/auth_helper.rb
+++ b/app/helpers/auth_helper.rb
@@ -24,6 +24,23 @@ module AuthHelper
     Gitlab::Auth::OAuth::Provider.label_for(name)
   end
 
+  def form_based_provider_priority
+    ['crowd', /^ldap/, 'kerberos']
+  end
+
+  def form_based_provider_with_highest_priority
+    @form_based_provider_with_highest_priority ||= begin
+      form_based_provider_priority.each do |provider_regexp|
+        highest_priority = form_based_providers.find {  |provider| provider.match?(provider_regexp) }
+        break highest_priority unless highest_priority.nil?
+      end
+    end
+  end
+
+  def form_based_auth_provider_has_active_class?(provider)
+    form_based_provider_with_highest_priority == provider
+  end
+
   def form_based_provider?(name)
     [LDAP_PROVIDER, 'crowd'].any? { |pattern| pattern === name.to_s }
   end
diff --git a/app/views/devise/shared/_signin_box.html.haml b/app/views/devise/shared/_signin_box.html.haml
index 5ddb3ece1cb..ec968e435cd 100644
--- a/app/views/devise/shared/_signin_box.html.haml
+++ b/app/views/devise/shared/_signin_box.html.haml
@@ -1,10 +1,10 @@
 - if form_based_providers.any?
   - if crowd_enabled?
-    .login-box.tab-pane.active{ id: "crowd", role: 'tabpanel' }
+    .login-box.tab-pane{ id: "crowd", role: 'tabpanel', class: active_when(form_based_auth_provider_has_active_class?(:crowd)) }
       .login-body
         = render 'devise/sessions/new_crowd'
   - @ldap_servers.each_with_index do |server, i|
-    .login-box.tab-pane{ id: "#{server['provider_name']}", role: 'tabpanel', class: active_when(i.zero? && !crowd_enabled?) }
+    .login-box.tab-pane{ id: "#{server['provider_name']}", role: 'tabpanel', class: active_when(i.zero? && form_based_auth_provider_has_active_class?(:ldapmain)) }
       .login-body
         = render 'devise/sessions/new_ldap', server: server
   - if password_authentication_enabled_for_web?
@@ -12,6 +12,8 @@
       .login-body
         = render 'devise/sessions/new_base'
 
+  = render_if_exists 'devise/sessions/new_smartcard'
+
 - elsif password_authentication_enabled_for_web?
   .login-box.tab-pane.active{ id: 'login-pane', role: 'tabpanel' }
     .login-body
diff --git a/app/views/devise/shared/_tabs_ldap.html.haml b/app/views/devise/shared/_tabs_ldap.html.haml
index 7dced0942f5..aee05b6c81c 100644
--- a/app/views/devise/shared/_tabs_ldap.html.haml
+++ b/app/views/devise/shared/_tabs_ldap.html.haml
@@ -1,10 +1,13 @@
 %ul.nav-links.new-session-tabs.nav-tabs.nav{ class: ('custom-provider-tabs' if form_based_providers.any?) }
   - if crowd_enabled?
     %li.nav-item
-      = link_to "Crowd", "#crowd", class: 'nav-link active', 'data-toggle' => 'tab'
+      = link_to "Crowd", "#crowd", class: "nav-link #{active_when(form_based_auth_provider_has_active_class?(:crowd))}", 'data-toggle' => 'tab'
   - @ldap_servers.each_with_index do |server, i|
     %li.nav-item
-      = link_to server['label'], "##{server['provider_name']}", class: "nav-link #{active_when(i.zero? && !crowd_enabled?)} qa-ldap-tab", 'data-toggle' => 'tab'
+      = link_to server['label'], "##{server['provider_name']}", class: "nav-link #{active_when(i.zero? && form_based_auth_provider_has_active_class?(:ldapmain))} qa-ldap-tab", 'data-toggle' => 'tab'
+
+  = render_if_exists 'devise/shared/tab_smartcard'
+
   - if password_authentication_enabled_for_web?
     %li.nav-item
       = link_to 'Standard', '#login-pane', class: 'nav-link qa-standard-tab', 'data-toggle' => 'tab'
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 4e91068ab88..efc3ce74627 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -650,7 +650,7 @@ describe ApplicationController do
   describe '#access_denied' do
     controller(described_class) do
       def index
-        access_denied!(params[:message])
+        access_denied!(params[:message], params[:status])
       end
     end
 
@@ -669,6 +669,12 @@ describe ApplicationController do
 
       expect(response).to have_gitlab_http_status(403)
     end
+
+    it 'renders a status passed to access denied' do
+      get :index, status: 401
+
+      expect(response).to have_gitlab_http_status(401)
+    end
   end
 
   context 'when invalid UTF-8 parameters are received' do
diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb
index 44758f862a8..ad856bd062e 100644
--- a/spec/features/users/login_spec.rb
+++ b/spec/features/users/login_spec.rb
@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe 'Login' do
   include TermsHelper
+  include UserLoginHelper
 
   before do
     stub_authentication_activity_metrics(debug: true)
@@ -546,29 +547,6 @@ describe 'Login' do
         ensure_tab_pane_correctness(false)
       end
     end
-
-    def ensure_tab_pane_correctness(visit_path = true)
-      if visit_path
-        visit new_user_session_path
-      end
-
-      ensure_tab_pane_counts
-      ensure_one_active_tab
-      ensure_one_active_pane
-    end
-
-    def ensure_tab_pane_counts
-      tabs_count = page.all('[role="tab"]').size
-      expect(page).to have_selector('[role="tabpanel"]', count: tabs_count)
-    end
-
-    def ensure_one_active_tab
-      expect(page).to have_selector('ul.new-session-tabs > li > a.active', count: 1)
-    end
-
-    def ensure_one_active_pane
-      expect(page).to have_selector('.tab-pane.active', count: 1)
-    end
   end
 
   context 'when terms are enforced' do
diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb
index 120b23e66ac..f0c2e4768ec 100644
--- a/spec/helpers/auth_helper_spec.rb
+++ b/spec/helpers/auth_helper_spec.rb
@@ -42,6 +42,16 @@ describe AuthHelper do
     end
   end
 
+  describe 'form_based_auth_provider_has_active_class?' do
+    it 'selects main LDAP server' do
+      allow(helper).to receive(:auth_providers) { [:twitter, :ldapprimary, :ldapsecondary, :kerberos] }
+      expect(helper.form_based_auth_provider_has_active_class?(:twitter)).to be(false)
+      expect(helper.form_based_auth_provider_has_active_class?(:ldapprimary)).to be(true)
+      expect(helper.form_based_auth_provider_has_active_class?(:ldapsecondary)).to be(false)
+      expect(helper.form_based_auth_provider_has_active_class?(:kerberos)).to be(false)
+    end
+  end
+
   describe 'enabled_button_based_providers' do
     before do
       allow(helper).to receive(:auth_providers) { [:twitter, :github] }
diff --git a/spec/support/helpers/user_login_helper.rb b/spec/support/helpers/user_login_helper.rb
new file mode 100644
index 00000000000..36c002f53af
--- /dev/null
+++ b/spec/support/helpers/user_login_helper.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module UserLoginHelper
+  def ensure_tab_pane_correctness(visit_path = true)
+    if visit_path
+      visit new_user_session_path
+    end
+
+    ensure_tab_pane_counts
+    ensure_one_active_tab
+    ensure_one_active_pane
+  end
+
+  def ensure_tab_pane_counts
+    tabs_count = page.all('[role="tab"]').size
+    expect(page).to have_selector('[role="tabpanel"]', count: tabs_count)
+  end
+
+  def ensure_one_active_tab
+    expect(page).to have_selector('ul.new-session-tabs > li > a.active', count: 1)
+  end
+
+  def ensure_one_active_pane
+    expect(page).to have_selector('.tab-pane.active', count: 1)
+  end
+end