summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorImre Farkas <ifarkas@gitlab.com>2018-11-12 22:40:42 +0100
committerImre Farkas <ifarkas@gitlab.com>2018-11-18 21:00:28 +0100
commit02326fa4b128c6272cc5c802cf5145f0fa6f6cc2 (patch)
treea17739647988e8f2c3e414cb9c91d76c09496f6a
parent29d8179ba07be3ed111f939285eb4064d84cb7df (diff)
downloadgitlab-ce-02326fa4b128c6272cc5c802cf5145f0fa6f6cc2.tar.gz
Backport of ee/8120: Smartcard authenticationif-ee-726-smartcard_auth-ce_port
-rw-r--r--app/controllers/application_controller.rb4
-rw-r--r--app/helpers/auth_helper.rb17
-rw-r--r--app/views/devise/shared/_signin_box.html.haml6
-rw-r--r--app/views/devise/shared/_tabs_ldap.html.haml7
-rw-r--r--spec/controllers/application_controller_spec.rb8
-rw-r--r--spec/features/users/login_spec.rb24
-rw-r--r--spec/helpers/auth_helper_spec.rb10
-rw-r--r--spec/support/helpers/user_login_helper.rb26
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