Commit 5bdc18c5 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'patch/fix-ldap-unblock-user-logic' into 'master'

Unblocks user when active_directory is disabled and it can be found

We implemented a specific block state to handle user blocking that originates from LDAP filtering rules / directory state in !2242. 

That introduced a regression in LDAP authentication when Active Directory support was disabled. You could have a scenario where the user would not be temporarily found (like a filtering rule), that would mark the user as `ldap_blocked`, but will never unblock it automatically when that state changed.

Fixes #14253, #13179, #13259, #13959

See merge request !3550
parents b6d5fcd4 5ee6bada
No related merge requests found
...@@ -33,7 +33,10 @@ module Gitlab ...@@ -33,7 +33,10 @@ module Gitlab
def allowed? def allowed?
if ldap_user if ldap_user
return true unless ldap_config.active_directory unless ldap_config.active_directory
user.activate if user.ldap_blocked?
return true
end
# Block user in GitLab if he/she was blocked in AD # 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) if Gitlab::LDAP::Person.disabled_via_active_directory?(user.ldap_identity.extern_uid, adapter)
......
...@@ -33,7 +33,7 @@ describe Gitlab::LDAP::Access, lib: true do ...@@ -33,7 +33,7 @@ describe Gitlab::LDAP::Access, lib: true do
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
it 'should block user in GitLab' do it 'blocks user in GitLab' do
access.allowed? access.allowed?
expect(user).to be_blocked expect(user).to be_blocked
expect(user).to be_ldap_blocked expect(user).to be_ldap_blocked
...@@ -78,6 +78,31 @@ describe Gitlab::LDAP::Access, lib: true do ...@@ -78,6 +78,31 @@ describe Gitlab::LDAP::Access, lib: true do
end end
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
context 'when user cannot be found' do
before do
allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(nil)
end
it { is_expected.to be_falsey }
it 'blocks user in GitLab' do
access.allowed?
expect(user).to be_blocked
expect(user).to be_ldap_blocked
end
end
context 'when user was previously ldap_blocked' do
before do
user.ldap_block
end
it 'unblocks the user if it exists' do
access.allowed?
expect(user).not_to be_blocked
end
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