Skip to content

Commit

Permalink
[api][webui] LDAP retries when connection fails.
Browse files Browse the repository at this point in the history
When the LDAP server closes the connection to the rails app,
UserLdapStrategy does not know about this. So when UserLdapStrategy
tries to connect with a closed connection, instead of re connecting it
just returns "Unauthorized". This change makes sure it tries to
reconnect once after getting a failed connection.
  • Loading branch information
evanrolfe committed Sep 21, 2017
1 parent 2b21f14 commit d78d046
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 23 deletions.
57 changes: 35 additions & 22 deletions src/api/app/models/user_ldap_strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,33 +197,46 @@ def self.find_with_ldap(login, password)
# @@ldap_search_con.bound? doesn't catch it as well. So when an error occurs, we
# simply it try it a seccond time, which forces the ldap connection to
# reinitialize (@@ldap_search_con is unbound and nil).
ldap_first_try = true
user = nil
user_filter = ''

if @@ldap_search_con.nil?
@@ldap_search_con = initialize_ldap_con(CONFIG['ldap_search_user'], CONFIG['ldap_search_auth'])
end
ldap_con = @@ldap_search_con
if ldap_con.nil?
Rails.logger.info("Unable to connect to LDAP server")
return
end
1.times do # Important! Do not remove this, this block may need to be re-run (by line 235)
if @@ldap_search_con.nil?
@@ldap_search_con = initialize_ldap_con(CONFIG['ldap_search_user'], CONFIG['ldap_search_auth'])
end

if CONFIG.has_key?('ldap_user_filter')
user_filter = "(&(#{CONFIG['ldap_search_attr']}=#{login})#{CONFIG['ldap_user_filter']})"
else
user_filter = "(#{CONFIG['ldap_search_attr']}=#{login})"
end
Rails.logger.debug("Search for #{CONFIG['ldap_search_base']} #{user_filter}")
begin
ldap_con.search(CONFIG['ldap_search_base'], LDAP::LDAP_SCOPE_SUBTREE, user_filter) do |entry|
user = entry.to_hash
ldap_con = @@ldap_search_con

if ldap_con.nil?
Rails.logger.info("Unable to connect to LDAP server")
return
end

if CONFIG.has_key?('ldap_user_filter')
user_filter = "(&(#{CONFIG['ldap_search_attr']}=#{login})#{CONFIG['ldap_user_filter']})"
else
user_filter = "(#{CONFIG['ldap_search_attr']}=#{login})"
end

Rails.logger.debug("Search for #{CONFIG['ldap_search_base']} #{user_filter}")

begin
ldap_con.search(CONFIG['ldap_search_base'], LDAP::LDAP_SCOPE_SUBTREE, user_filter) do |entry|
user = entry.to_hash
end
rescue StandardError
Rails.logger.info("Search failed: error #{ @@ldap_search_con.err}: #{ @@ldap_search_con.err2string(@@ldap_search_con.err)}")
@@ldap_search_con.unbind
@@ldap_search_con = nil

if ldap_first_try
ldap_first_try = false
redo
end

return
end
rescue StandardError
Rails.logger.info("Search failed: error #{ @@ldap_search_con.err}: #{ @@ldap_search_con.err2string(@@ldap_search_con.err)}")
@@ldap_search_con.unbind
@@ldap_search_con = nil
return
end

if user.nil?
Expand Down
35 changes: 35 additions & 0 deletions src/api/spec/models/user_ldap_strategy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,41 @@
is_expected.to eq(['John', 'S'])
end
end

# This particular case occurs when a connection is made to the server, and stored in
# @@ldap_search_con but then the server closes the connection. UserLdapStrategy has no way of
# knowing if the connection was closed by the server so we need to make sure that
# UserLdapStrategy attempts to reconnect.
context 'when the connection is closed by the server' do
include_context 'setup ldap mock with user mock'
include_context 'an ldap connection'
include_context 'mock searching a user' do
let(:ldap_user) { double(:ldap_user, to_hash: { 'dn' => 'tux', 'sn' => ['John', 'Smith'] }) }
end

before do
times_called = 0
# First time LDAP works, second time is raises an error, third time it works etc.
allow(ldap_mock).to receive(:search) do
raise StandardError if times_called == 1
times_called += 1
end
allow(ldap_mock).to receive(:err).and_return('something went wrong')
allow(ldap_mock).to receive(:err2string).and_return('something went wrong')
allow(ldap_mock).to receive(:unbind)
# This connects to LDAP and stores the connection in a class var
UserLdapStrategy.find_with_ldap('tux', 'tux_password')
end

subject! do
# This attempts to use the LDAP connection which already exists in the class var
UserLdapStrategy.find_with_ldap('tux', 'tux_password')
end

it 'returns nil' do
is_expected.to eq(['John', 'tux'])
end
end
end
end
end
2 changes: 1 addition & 1 deletion src/api/test/unit/code_quality_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def setup
'SearchController#search' => 81.15,
'SourceController#project_command_copy' => 140.04,
'SourceController#update_project_meta' => 98.09,
'UserLdapStrategy::find_with_ldap' => 87.47,
'UserLdapStrategy::find_with_ldap' => 94.35,
'User::find_with_credentials_via_ldap' => 88.93,
'UserLdapStrategy::render_grouplist_ldap' => 89.86,
'Webui::DriverUpdateController#save' => 91.69,
Expand Down

0 comments on commit d78d046

Please sign in to comment.