Commit 49f51ff1 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'optimize_ldap' into 'master'

Optimize LDAP and add a search timeout

Related to #4282 

This merge request arranges some things in `access.rb` to facilitate some optimizations in EE (to come later). It also adds a 10 second timeout to all LDAP searches so the entire worker is not blocked if some query doesn't return in a reasonable amount of time. This timeout is configurable per LDAP server.

See merge request !2267
parents d65e7aa9 67aa0b8c
...@@ -37,6 +37,7 @@ v 8.4.0 (unreleased) ...@@ -37,6 +37,7 @@ v 8.4.0 (unreleased)
v 8.3.3 (unreleased) v 8.3.3 (unreleased)
- Preserve CE behavior with JIRA integration by only calling API if URL is set - Preserve CE behavior with JIRA integration by only calling API if URL is set
- Fix duplicated branch creation/deletion events when using Web UI (Stan Hu) - Fix duplicated branch creation/deletion events when using Web UI (Stan Hu)
- Add configurable LDAP server query timeout
- Get "Merge when build succeeds" to work when commits were pushed to MR target branch while builds were running - Get "Merge when build succeeds" to work when commits were pushed to MR target branch while builds were running
- Suppress e-mails on failed builds if allow_failure is set (Stan Hu) - Suppress e-mails on failed builds if allow_failure is set (Stan Hu)
- Fix project transfer e-mail sending incorrect paths in e-mail notification (Stan Hu) - Fix project transfer e-mail sending incorrect paths in e-mail notification (Stan Hu)
......
...@@ -204,6 +204,11 @@ production: &base ...@@ -204,6 +204,11 @@ production: &base
bind_dn: '_the_full_dn_of_the_user_you_will_bind_with' bind_dn: '_the_full_dn_of_the_user_you_will_bind_with'
password: '_the_password_of_the_bind_user' password: '_the_password_of_the_bind_user'
# Set a timeout, in seconds, for LDAP queries. This helps avoid blocking
# a request if the LDAP server becomes unresponsive.
# A value of 0 means there is no timeout.
timeout: 10
# This setting specifies if LDAP server is Active Directory LDAP server. # This setting specifies if LDAP server is Active Directory LDAP server.
# For non AD servers it skips the AD specific queries. # For non AD servers it skips the AD specific queries.
# If your LDAP server is not AD, set this to false. # If your LDAP server is not AD, set this to false.
......
...@@ -108,6 +108,7 @@ if Settings.ldap['enabled'] || Rails.env.test? ...@@ -108,6 +108,7 @@ if Settings.ldap['enabled'] || Rails.env.test?
Settings.ldap['servers'].each do |key, server| Settings.ldap['servers'].each do |key, server|
server['label'] ||= 'LDAP' server['label'] ||= 'LDAP'
server['timeout'] ||= 10.seconds
server['block_auto_created_users'] = false if server['block_auto_created_users'].nil? server['block_auto_created_users'] = false if server['block_auto_created_users'].nil?
server['allow_username_or_email_login'] = false if server['allow_username_or_email_login'].nil? server['allow_username_or_email_login'] = false if server['allow_username_or_email_login'].nil?
server['active_directory'] = true if server['active_directory'].nil? server['active_directory'] = true if server['active_directory'].nil?
......
...@@ -48,6 +48,11 @@ main: # 'main' is the GitLab 'provider ID' of this LDAP server ...@@ -48,6 +48,11 @@ main: # 'main' is the GitLab 'provider ID' of this LDAP server
bind_dn: '_the_full_dn_of_the_user_you_will_bind_with' bind_dn: '_the_full_dn_of_the_user_you_will_bind_with'
password: '_the_password_of_the_bind_user' password: '_the_password_of_the_bind_user'
# Set a timeout, in seconds, for LDAP queries. This helps avoid blocking
# a request if the LDAP server becomes unresponsive.
# A value of 0 means there is no timeout.
timeout: 10
# This setting specifies if LDAP server is Active Directory LDAP server. # This setting specifies if LDAP server is Active Directory LDAP server.
# For non AD servers it skips the AD specific queries. # For non AD servers it skips the AD specific queries.
# If your LDAP server is not AD, set this to false. # If your LDAP server is not AD, set this to false.
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
module Gitlab module Gitlab
module LDAP module LDAP
class Access class Access
attr_reader :adapter, :provider, :user attr_reader :provider, :user
def self.open(user, &block) def self.open(user, &block)
Gitlab::LDAP::Adapter.open(user.ldap_identity.provider) do |adapter| Gitlab::LDAP::Adapter.open(user.ldap_identity.provider) do |adapter|
...@@ -32,7 +32,7 @@ module Gitlab ...@@ -32,7 +32,7 @@ module Gitlab
end end
def allowed? def allowed?
if Gitlab::LDAP::Person.find_by_dn(user.ldap_identity.extern_uid, adapter) if ldap_user
return true unless ldap_config.active_directory return true unless ldap_config.active_directory
# Block user in GitLab if he/she was blocked in AD # Block user in GitLab if he/she was blocked in AD
...@@ -59,6 +59,10 @@ module Gitlab ...@@ -59,6 +59,10 @@ module Gitlab
def ldap_config def ldap_config
Gitlab::LDAP::Config.new(provider) Gitlab::LDAP::Config.new(provider)
end end
def ldap_user
@ldap_user ||= Gitlab::LDAP::Person.find_by_dn(user.ldap_identity.extern_uid, adapter)
end
end end
end end
end end
...@@ -70,19 +70,25 @@ module Gitlab ...@@ -70,19 +70,25 @@ module Gitlab
end end
def ldap_search(*args) def ldap_search(*args)
results = ldap.search(*args) # Net::LDAP's `time` argument doesn't work. Use Ruby `Timeout` instead.
Timeout.timeout(config.timeout) do
results = ldap.search(*args)
if results.nil? if results.nil?
response = ldap.get_operation_result response = ldap.get_operation_result
unless response.code.zero? unless response.code.zero?
Rails.logger.warn("LDAP search error: #{response.message}") Rails.logger.warn("LDAP search error: #{response.message}")
end end
[] []
else else
results results
end
end end
rescue Timeout::Error
Rails.logger.warn("LDAP search timed out after #{config.timeout} seconds")
[]
end end
end end
end end
......
...@@ -88,6 +88,10 @@ module Gitlab ...@@ -88,6 +88,10 @@ module Gitlab
options['attributes'] options['attributes']
end end
def timeout
options['timeout'].to_i
end
protected protected
def base_config def base_config
Gitlab.config.ldap Gitlab.config.ldap
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment