Commit 1be2ec2d authored by Stan Hu's avatar Stan Hu

Fix system hook not firing for blocked users when LDAP sign-in is used

An LDAP sign-in request results in a different request parameter than
a standard GitLab sign-in. Since Warden doesn't pass us the user that
was blocked, we first search for a `username` in the request parameters
and then look for `user.login`.

Closes #46307
parent 40683268
---
title: Fix system hook not firing for blocked users when LDAP sign-in is used
merge_request:
author:
type: fixed
...@@ -17,7 +17,9 @@ module Gitlab ...@@ -17,7 +17,9 @@ module Gitlab
# message passed along by Warden. # message passed along by Warden.
return unless message == User::BLOCKED_MESSAGE 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? return unless login.present?
......
...@@ -17,12 +17,8 @@ describe Gitlab::Auth::BlockedUserTracker do ...@@ -17,12 +17,8 @@ describe Gitlab::Auth::BlockedUserTracker do
end end
context 'failed login due to blocked user' do context 'failed login due to blocked user' do
let(:env) do let(:base_env) { { 'warden.options' => { message: User::BLOCKED_MESSAGE } } }
{ let(:env) { base_env.merge(request_env) }
'warden.options' => { message: User::BLOCKED_MESSAGE },
described_class::ACTIVE_RECORD_REQUEST_PARAMS => { 'user' => { 'login' => user.username } }
}
end
subject { described_class.log_if_user_blocked(env) } subject { described_class.log_if_user_blocked(env) }
...@@ -30,23 +26,37 @@ describe Gitlab::Auth::BlockedUserTracker do ...@@ -30,23 +26,37 @@ describe Gitlab::Auth::BlockedUserTracker do
expect_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).with(user, :failed_login) expect_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).with(user, :failed_login)
end end
it 'logs a blocked user' do context 'via GitLab login' do
user.block! let(:request_env) { { described_class::ACTIVE_RECORD_REQUEST_PARAMS => { 'user' => { 'login' => user.username } } } }
expect(subject).to be_truthy it 'logs a blocked user' do
end user.block!
expect(subject).to be_truthy
end
it 'logs a blocked user by e-mail' do it 'logs a blocked user by e-mail' do
user.block! user.block!
env[described_class::ACTIVE_RECORD_REQUEST_PARAMS]['user']['login'] = user.email env[described_class::ACTIVE_RECORD_REQUEST_PARAMS]['user']['login'] = user.email
expect(subject).to be_truthy expect(subject).to be_truthy
end
end end
it 'logs a LDAP blocked user' do context 'via LDAP login' do
user.ldap_block! 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 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