Commit 68364fe2 authored by Markus Koller's avatar Markus Koller Committed by Rémy Coutable

Log LDAP lookup errors and don't swallow unrelated exceptions

Signed-off-by: default avatarRoger Meier <r.meier@siemens.com>
parent 3b206ccb
...@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 8.13.0 (unreleased) v 8.13.0 (unreleased)
- Use gitlab-shell v3.6.2 (GIT TRACE logging) - Use gitlab-shell v3.6.2 (GIT TRACE logging)
- Speed-up group milestones show page - Speed-up group milestones show page
- Log LDAP lookup errors and don't swallow unrelated exceptions. !6103 (Markus Koller)
- Add more tests for calendar contribution (ClemMakesApps) - Add more tests for calendar contribution (ClemMakesApps)
- Fix robots.txt disallowing access to groups starting with "s" (Matt Harrison) - Fix robots.txt disallowing access to groups starting with "s" (Matt Harrison)
- Only update issuable labels if they have been changed - Only update issuable labels if they have been changed
......
...@@ -275,3 +275,9 @@ If you are getting 'Connection Refused' errors when trying to connect to the ...@@ -275,3 +275,9 @@ If you are getting 'Connection Refused' errors when trying to connect to the
LDAP server please double-check the LDAP `port` and `method` settings used by LDAP server please double-check the LDAP `port` and `method` settings used by
GitLab. Common combinations are `method: 'plain'` and `port: 389`, OR GitLab. Common combinations are `method: 'plain'` and `port: 389`, OR
`method: 'ssl'` and `port: 636`. `method: 'ssl'` and `port: 636`.
### Login with valid credentials rejected
If there is an unexpected error while authenticating the user with the LDAP
backend, the login is rejected and details about the error are logged to
`production.log`.
...@@ -51,8 +51,6 @@ module Gitlab ...@@ -51,8 +51,6 @@ module Gitlab
user.ldap_block user.ldap_block
false false
end end
rescue
false
end end
def adapter def adapter
......
...@@ -62,6 +62,9 @@ module Gitlab ...@@ -62,6 +62,9 @@ module Gitlab
results results
end end
end end
rescue Net::LDAP::Error => error
Rails.logger.warn("LDAP search raised exception #{error.class}: #{error.message}")
[]
rescue Timeout::Error rescue Timeout::Error
Rails.logger.warn("LDAP search timed out after #{config.timeout} seconds") Rails.logger.warn("LDAP search timed out after #{config.timeout} seconds")
[] []
......
...@@ -73,17 +73,33 @@ describe Gitlab::LDAP::Adapter, lib: true do ...@@ -73,17 +73,33 @@ describe Gitlab::LDAP::Adapter, lib: true do
describe '#dn_matches_filter?' do describe '#dn_matches_filter?' do
subject { adapter.dn_matches_filter?(:dn, :filter) } subject { adapter.dn_matches_filter?(:dn, :filter) }
context "when the search result is non-empty" do
before { allow(adapter).to receive(:ldap_search).and_return([:foo]) }
it { is_expected.to be_truthy }
end
context "when the search result is empty" do
before { allow(adapter).to receive(:ldap_search).and_return([]) }
it { is_expected.to be_falsey }
end
end
describe '#ldap_search' do
subject { adapter.ldap_search(base: :dn, filter: :filter) }
context "when the search is successful" do context "when the search is successful" do
context "and the result is non-empty" do context "and the result is non-empty" do
before { allow(ldap).to receive(:search).and_return([:foo]) } before { allow(ldap).to receive(:search).and_return([:foo]) }
it { is_expected.to be_truthy } it { is_expected.to eq [:foo] }
end end
context "and the result is empty" do context "and the result is empty" do
before { allow(ldap).to receive(:search).and_return([]) } before { allow(ldap).to receive(:search).and_return([]) }
it { is_expected.to be_falsey } it { is_expected.to eq [] }
end end
end end
...@@ -95,7 +111,22 @@ describe Gitlab::LDAP::Adapter, lib: true do ...@@ -95,7 +111,22 @@ describe Gitlab::LDAP::Adapter, lib: true do
) )
end end
it { is_expected.to be_falsey } it { is_expected.to eq [] }
end
context "when the search raises an LDAP exception" do
before do
allow(ldap).to receive(:search) { raise Net::LDAP::Error, "some error" }
allow(Rails.logger).to receive(:warn)
end
it { is_expected.to eq [] }
it 'logs the error' do
subject
expect(Rails.logger).to have_received(:warn).with(
"LDAP search raised exception Net::LDAP::Error: some error")
end
end end
end end
end end
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