Commit a4d0d6f2 authored by Drew Blessing's avatar Drew Blessing

Fix LDAP DN case-mismatch bug in LDAP group sync

In a few circumstances, we saw that the LDAP person would
present the DN in one mix of case while a given LDAP group
member DN list would return the DN in another mix of case.
LDAP group sync was unable to make the match between these
members and it caused created/updated timestamp to be reset
and LDAP overrides to be ignored.
parent 97437e88
---
title: Fix LDAP DN case-mismatch bug in LDAP group sync
merge_request: 1337
author:
...@@ -7,10 +7,10 @@ module EE ...@@ -7,10 +7,10 @@ module EE
class AccessLevels < Hash class AccessLevels < Hash
def set(dns, to:) def set(dns, to:)
dns.each do |dn| dns.each do |dn|
current = self[dn] current = self[dn.downcase]
# Keep the higher of the access values. # Keep the higher of the access values.
self[dn] = to if current.nil? || to > current self[dn.downcase] = to if current.nil? || to > current
end end
end end
end end
......
...@@ -103,7 +103,7 @@ module EE ...@@ -103,7 +103,7 @@ module EE
user = member.user user = member.user
identity = user.identities.select(:id, :extern_uid) identity = user.identities.select(:id, :extern_uid)
.with_provider(provider).first .with_provider(provider).first
member_dn = identity.extern_uid member_dn = identity.extern_uid.downcase
# Skip if this is not an LDAP user with a valid `extern_uid`. # Skip if this is not an LDAP user with a valid `extern_uid`.
next unless member_dn.present? next unless member_dn.present?
......
...@@ -126,7 +126,8 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do ...@@ -126,7 +126,8 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do
end end
let(:group) do let(:group) do
create(:group_with_ldap_group_link, :access_requestable, create(:group_with_ldap_group_link,
:access_requestable,
cn: 'ldap_group1', cn: 'ldap_group1',
group_access: ::Gitlab::Access::DEVELOPER) group_access: ::Gitlab::Access::DEVELOPER)
end end
...@@ -253,6 +254,42 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do ...@@ -253,6 +254,42 @@ describe EE::Gitlab::LDAP::Sync::Group, lib: true do
.to eq([::Gitlab::Access::DEVELOPER, ::Gitlab::Access::OWNER]) .to eq([::Gitlab::Access::DEVELOPER, ::Gitlab::Access::OWNER])
end end
end end
context 'when the extern_uid and group member DNs have different case' do
let(:user1) { create(:user) }
let(:user2) { create(:user) }
# Change the case once on the LDAP group, and once on the GitLab Identity
# to test that both sides can handle the differing case.
let(:ldap_group1) do
ldap_group_entry(%W(
#{user_dn(user1.username).upcase}
#{user_dn(user2.username)}
))
end
it 'does not revert the overrides' do
create(:identity, user: user1, extern_uid: user_dn(user1.username))
create(:identity, user: user2, extern_uid: user_dn(user2.username).upcase)
group.members.create(
user: user1,
access_level: ::Gitlab::Access::MASTER,
ldap: true,
override: true
)
group.members.create(
user: user2,
access_level: ::Gitlab::Access::OWNER,
ldap: true,
override: true
)
sync_group.update_permissions
expect(group.members.pluck(:access_level))
.to match_array([::Gitlab::Access::MASTER, ::Gitlab::Access::OWNER])
end
end
end end
# Test that membership can be resolved for all different type of LDAP groups # Test that membership can be resolved for all different type of LDAP groups
......
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