diff --git a/CHANGELOG-EE b/CHANGELOG-EE index 7e087b5314..35c3449d61 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -1,3 +1,6 @@ +v 6.7.0 + - Improve LDAP sign-in speed by reusing connections + v 6.5.0 - Add reset permissions button to Group#members page diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 11a8f7155c..9574685b1c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -183,14 +183,16 @@ class ApplicationController < ActionController::Base def ldap_security_check if current_user && current_user.ldap_user? && current_user.requires_ldap_check? - if gitlab_ldap_access.allowed?(current_user) - gitlab_ldap_access.update_permissions(current_user) - current_user.last_credential_check_at = Time.now - current_user.save - else - sign_out current_user - flash[:alert] = "Access denied for your LDAP account." - redirect_to new_user_session_path + gitlab_ldap_access do |access| + if access.allowed?(current_user) + access.update_permissions(current_user) + current_user.last_credential_check_at = Time.now + current_user.save + else + sign_out current_user + flash[:alert] = "Access denied for your LDAP account." + redirect_to new_user_session_path + end end end end @@ -200,8 +202,8 @@ class ApplicationController < ActionController::Base @event_filter ||= EventFilter.new(filters) end - def gitlab_ldap_access - Gitlab::LDAP::Access.new + def gitlab_ldap_access(&block) + Gitlab::LDAP::Access.open { |access| block.call(access) } end # JSON for infinite scroll via Pager object diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index c6b9792be4..f53b753733 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -21,12 +21,14 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController @user = Gitlab::LDAP::User.find_or_create(oauth) @user.remember_me = true if @user.persisted? - if gitlab_ldap_access.allowed?(@user) - gitlab_ldap_access.update_permissions(@user) - sign_in_and_redirect(@user) - else - flash[:alert] = "Access denied for your LDAP account." - redirect_to new_user_session_path + gitlab_ldap_access do |access| + if access.allowed?(@user) + access.update_permissions(@user) + sign_in_and_redirect(@user) + else + flash[:alert] = "Access denied for your LDAP account." + redirect_to new_user_session_path + end end end diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index a36bb3ea11..5a4add79c8 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -7,8 +7,20 @@ module Gitlab module LDAP class Access + attr_reader :adapter + + def self.open(&block) + Gitlab::LDAP::Adapter.open do |adapter| + block.call(self.new(adapter)) + end + end + + def initialize(adapter=nil) + @adapter = adapter + end + def allowed?(user) - !!Gitlab::LDAP::Person.find_by_dn(user.extern_uid) + !!Gitlab::LDAP::Person.find_by_dn(user.extern_uid, adapter) rescue false end @@ -19,13 +31,13 @@ module Gitlab return true unless Gitlab.config.ldap['group_base'].present? # Get LDAP user entry - ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid) + ldap_user = Gitlab::LDAP::Person.find_by_dn(user.extern_uid, adapter) # Get all GitLab groups with activated LDAP groups = ::Group.where('ldap_cn IS NOT NULL') # Get LDAP groups based on cn from GitLab groups - ldap_groups = groups.pluck(:ldap_cn).map { |cn| Gitlab::LDAP::Group.find_by_cn(cn) } + ldap_groups = groups.pluck(:ldap_cn).map { |cn| Gitlab::LDAP::Group.find_by_cn(cn, adapter) } ldap_groups = ldap_groups.compact.uniq # Iterate over ldap groups and check user membership diff --git a/lib/gitlab/ldap/adapter.rb b/lib/gitlab/ldap/adapter.rb index 62d2186329..901e50c471 100644 --- a/lib/gitlab/ldap/adapter.rb +++ b/lib/gitlab/ldap/adapter.rb @@ -9,7 +9,17 @@ module Gitlab class Adapter attr_reader :ldap - def initialize + def self.open(&block) + Net::LDAP.open(adapter_options) do |ldap| + block.call(self.new(ldap)) + end + end + + def self.config + Gitlab.config.ldap + end + + def self.adapter_options encryption = config['method'].to_s == 'ssl' ? :simple_tls : nil options = { @@ -29,8 +39,12 @@ module Gitlab if config['password'] || config['bind_dn'] options.merge!(auth_options) end + options + end - @ldap = Net::LDAP.new(options) + + def initialize(ldap=nil) + @ldap = ldap || Net::LDAP.new(self.class.adapter_options) end # Get LDAP groups from ou=Groups @@ -95,7 +109,7 @@ module Gitlab private def config - @config ||= Gitlab.config.ldap + @config ||= self.class.config end end end diff --git a/lib/gitlab/ldap/group.rb b/lib/gitlab/ldap/group.rb index 61cab8393c..b7af14e225 100644 --- a/lib/gitlab/ldap/group.rb +++ b/lib/gitlab/ldap/group.rb @@ -7,8 +7,9 @@ module Gitlab module LDAP class Group - def self.find_by_cn(cn) - Gitlab::LDAP::Adapter.new.group(cn) + def self.find_by_cn(cn, adapter=nil) + adapter ||= Gitlab::LDAP::Adapter.new + adapter.group(cn) end def initialize(entry) diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb index 339e6f37e2..73203468a6 100644 --- a/lib/gitlab/ldap/person.rb +++ b/lib/gitlab/ldap/person.rb @@ -7,12 +7,14 @@ module Gitlab module LDAP class Person - def self.find_by_uid(uid) - Gitlab::LDAP::Adapter.new.user(config.uid, uid) + def self.find_by_uid(uid, adapter=nil) + adapter ||= Gitlab::LDAP::Adapter.new + adapter.user(config.uid, uid) end - def self.find_by_dn(dn) - Gitlab::LDAP::Adapter.new.user('dn', dn) + def self.find_by_dn(dn, adapter=nil) + adapter ||= Gitlab::LDAP::Adapter.new + adapter.user('dn', dn) end def initialize(entry)