diff options
| -rw-r--r-- | changelogs/unreleased/sh-fix-blocked-user-account-ldap.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/auth/blocked_user_tracker.rb | 4 | ||||
| -rw-r--r-- | spec/lib/gitlab/auth/blocked_user_tracker_spec.rb | 44 |
3 files changed, 35 insertions, 18 deletions
diff --git a/changelogs/unreleased/sh-fix-blocked-user-account-ldap.yml b/changelogs/unreleased/sh-fix-blocked-user-account-ldap.yml new file mode 100644 index 00000000000..f7abe763ea8 --- /dev/null +++ b/changelogs/unreleased/sh-fix-blocked-user-account-ldap.yml @@ -0,0 +1,5 @@ +--- +title: Fix system hook not firing for blocked users when LDAP sign-in is used +merge_request: +author: +type: fixed diff --git a/lib/gitlab/auth/blocked_user_tracker.rb b/lib/gitlab/auth/blocked_user_tracker.rb index dae03a179e4..7609a7b04f6 100644 --- a/lib/gitlab/auth/blocked_user_tracker.rb +++ b/lib/gitlab/auth/blocked_user_tracker.rb @@ -17,7 +17,9 @@ module Gitlab # message passed along by Warden. return unless message == User::BLOCKED_MESSAGE - login = env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login') + # Check for either LDAP or regular GitLab account logins + login = env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'username') || + env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login') return unless login.present? diff --git a/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb index 726a3c1c83a..43b68e69131 100644 --- a/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb +++ b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb @@ -17,12 +17,8 @@ describe Gitlab::Auth::BlockedUserTracker do end context 'failed login due to blocked user' do - let(:env) do - { - 'warden.options' => { message: User::BLOCKED_MESSAGE }, - described_class::ACTIVE_RECORD_REQUEST_PARAMS => { 'user' => { 'login' => user.username } } - } - end + let(:base_env) { { 'warden.options' => { message: User::BLOCKED_MESSAGE } } } + let(:env) { base_env.merge(request_env) } subject { described_class.log_if_user_blocked(env) } @@ -30,23 +26,37 @@ describe Gitlab::Auth::BlockedUserTracker do expect_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).with(user, :failed_login) end - it 'logs a blocked user' do - user.block! + context 'via GitLab login' do + let(:request_env) { { described_class::ACTIVE_RECORD_REQUEST_PARAMS => { 'user' => { 'login' => user.username } } } } - expect(subject).to be_truthy - end + it 'logs a blocked user' do + user.block! + + expect(subject).to be_truthy + end - it 'logs a blocked user by e-mail' do - user.block! - env[described_class::ACTIVE_RECORD_REQUEST_PARAMS]['user']['login'] = user.email + it 'logs a blocked user by e-mail' do + user.block! + env[described_class::ACTIVE_RECORD_REQUEST_PARAMS]['user']['login'] = user.email - expect(subject).to be_truthy + expect(subject).to be_truthy + end end - it 'logs a LDAP blocked user' do - user.ldap_block! + context 'via LDAP login' do + let(:request_env) { { described_class::ACTIVE_RECORD_REQUEST_PARAMS => { 'username' => user.username } } } + + it 'logs a blocked user' do + user.block! + + expect(subject).to be_truthy + end + + it 'logs a LDAP blocked user' do + user.ldap_block! - expect(subject).to be_truthy + expect(subject).to be_truthy + end end end end |