diff options
author | Rémy Coutable <remy@rymai.me> | 2017-01-09 16:39:19 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2017-01-09 16:39:19 +0300 |
commit | 60569032658e93658f789f8bd8bbaef56ea39412 (patch) | |
tree | 4b83f5448114c44daa8f4805ba737803bb1d52ca | |
parent | a3d9e4fd883005ef6a840701826e79f4abf2b637 (diff) | |
parent | bd0c171c55d974887e598bc63b2625810629bea2 (diff) |
Merge branch 'feature/log-ldap-to-application-log' into 'master'
Log LDAP blocking/unblocking events to application log
See merge request !8042
-rw-r--r-- | changelogs/unreleased/feature-log-ldap-to-application-log.yml | 4 | ||||
-rw-r--r-- | doc/administration/auth/ldap.md | 9 | ||||
-rw-r--r-- | lib/gitlab/ldap/access.rb | 26 | ||||
-rw-r--r-- | spec/lib/gitlab/ldap/access_spec.rb | 63 |
4 files changed, 87 insertions, 15 deletions
diff --git a/changelogs/unreleased/feature-log-ldap-to-application-log.yml b/changelogs/unreleased/feature-log-ldap-to-application-log.yml new file mode 100644 index 00000000000..4cfbc23edb7 --- /dev/null +++ b/changelogs/unreleased/feature-log-ldap-to-application-log.yml @@ -0,0 +1,4 @@ +--- +title: Log LDAP blocking/unblocking events to application log +merge_request: 8042 +author: Markus Koller diff --git a/doc/administration/auth/ldap.md b/doc/administration/auth/ldap.md index b8b63df091e..04723365277 100644 --- a/doc/administration/auth/ldap.md +++ b/doc/administration/auth/ldap.md @@ -298,8 +298,11 @@ LDAP server please double-check the LDAP `port` and `method` settings used by GitLab. Common combinations are `method: 'plain'` and `port: 389`, OR `method: 'ssl'` and `port: 636`. -### Login with valid credentials rejected +### Troubleshooting -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 +If a user account is blocked or unblocked due to the LDAP configuration, a +message will be logged to `application.log`. + +If there is an unexpected error during an LDAP lookup (configuration error, +timeout), the login is rejected and a message will be logged to `production.log`. diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index 7e06bd2b0fb..7ed01bf56ca 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -34,21 +34,21 @@ module Gitlab def allowed? if ldap_user unless ldap_config.active_directory - user.activate if user.ldap_blocked? + unblock_user(user, 'is available again') if user.ldap_blocked? return true end # Block user in GitLab if he/she was blocked in AD if Gitlab::LDAP::Person.disabled_via_active_directory?(user.ldap_identity.extern_uid, adapter) - user.ldap_block + block_user(user, 'is disabled in Active Directory') false else - user.activate if user.ldap_blocked? + unblock_user(user, 'is not disabled anymore') if user.ldap_blocked? true end else # Block the user if they no longer exist in LDAP/AD - user.ldap_block + block_user(user, 'does not exist anymore') false end end @@ -64,6 +64,24 @@ module Gitlab def ldap_user @ldap_user ||= Gitlab::LDAP::Person.find_by_dn(user.ldap_identity.extern_uid, adapter) end + + def block_user(user, reason) + user.ldap_block + + Gitlab::AppLogger.info( + "LDAP account \"#{user.ldap_identity.extern_uid}\" #{reason}, " + + "blocking Gitlab user \"#{user.name}\" (#{user.email})" + ) + end + + def unblock_user(user, reason) + user.activate + + Gitlab::AppLogger.info( + "LDAP account \"#{user.ldap_identity.extern_uid}\" #{reason}, " + + "unblocking Gitlab user \"#{user.name}\" (#{user.email})" + ) + end end end end diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 534bcbf39fe..011c33e63a1 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -15,9 +15,9 @@ describe Gitlab::LDAP::Access, lib: true do it { is_expected.to be_falsey } it 'should block user in GitLab' do + expect(access).to receive(:block_user).with(user, 'does not exist anymore') + access.allowed? - expect(user).to be_blocked - expect(user).to be_ldap_blocked end end @@ -34,9 +34,9 @@ describe Gitlab::LDAP::Access, lib: true do it { is_expected.to be_falsey } it 'blocks user in GitLab' do + expect(access).to receive(:block_user).with(user, 'is disabled in Active Directory') + access.allowed? - expect(user).to be_blocked - expect(user).to be_ldap_blocked end end @@ -53,7 +53,10 @@ describe Gitlab::LDAP::Access, lib: true do end it 'does not unblock user in GitLab' do + expect(access).not_to receive(:unblock_user) + access.allowed? + expect(user).to be_blocked expect(user).not_to be_ldap_blocked # this block is handled by omniauth not by our internal logic end @@ -65,8 +68,9 @@ describe Gitlab::LDAP::Access, lib: true do end it 'unblocks user in GitLab' do + expect(access).to receive(:unblock_user).with(user, 'is not disabled anymore') + access.allowed? - expect(user).not_to be_blocked end end end @@ -87,9 +91,9 @@ describe Gitlab::LDAP::Access, lib: true do it { is_expected.to be_falsey } it 'blocks user in GitLab' do + expect(access).to receive(:block_user).with(user, 'does not exist anymore') + access.allowed? - expect(user).to be_blocked - expect(user).to be_ldap_blocked end end @@ -99,11 +103,54 @@ describe Gitlab::LDAP::Access, lib: true do end it 'unblocks the user if it exists' do + expect(access).to receive(:unblock_user).with(user, 'is available again') + access.allowed? - expect(user).not_to be_blocked end end end end end + + describe '#block_user' do + before do + user.activate + allow(Gitlab::AppLogger).to receive(:info) + + access.block_user user, 'reason' + end + + it 'blocks the user' do + expect(user).to be_blocked + expect(user).to be_ldap_blocked + end + + it 'logs the reason' do + expect(Gitlab::AppLogger).to have_received(:info).with( + "LDAP account \"123456\" reason, " + + "blocking Gitlab user \"#{user.name}\" (#{user.email})" + ) + end + end + + describe '#unblock_user' do + before do + user.ldap_block + allow(Gitlab::AppLogger).to receive(:info) + + access.unblock_user user, 'reason' + end + + it 'activates the user' do + expect(user).not_to be_blocked + expect(user).not_to be_ldap_blocked + end + + it 'logs the reason' do + Gitlab::AppLogger.info( + "LDAP account \"123456\" reason, " + + "unblocking Gitlab user \"#{user.name}\" (#{user.email})" + ) + end + end end |