Merge branch 'mk-fix-issue-3854' into 'master'

Fix LDAP group sync for nested groups

Closes #3854

See merge request gitlab-org/gitlab-ee!3217

changelogs/unreleased-ee/mk-fix-issue-3854.yml

+---
+title: Fix LDAP group sync for nested groups e.g. when base has uppercase or extraneous spaces
+merge_request: 3217
+author:
+type: fixed

lib/gitlab/ldap/config.rb

@@ -89,7 +89,7 @@ module Gitlab
       end
 
       def base
-        options['base']
+        @base ||= Person.normalize_dn(options['base'])
       end
 
       def uid

spec/ee/spec/lib/ee/gitlab/ldap/group_spec.rb

@@ -163,6 +163,25 @@ describe EE::Gitlab::LDAP::Group do
           .to receive(:warn).with(/Received invalid member/)
         expect(group.member_dns).not_to include('invalid,ou=user,ou=groups,dc=example,dc=com')
       end
+
+      it 'resolves the correct member_dns when the LDAP base is not normalized' do
+        # E.g. When `base` has uppercase characters and extraneous spaces.
+        # Stub looks different because `LDAP#Config#base` must be exercised.
+        stub_ldap_config(options: { 'base' => 'DC=example, DC= com' })
+
+        nested_groups = [group2_entry]
+        stub_ldap_adapter_nested_groups(group.dn, nested_groups, adapter)
+        stub_ldap_adapter_nested_groups(group2_entry.dn, [], adapter)
+
+        expect(group.member_dns).to match_array(
+          %w(
+            uid=user1,ou=users,dc=example,dc=com
+            uid=user2,ou=users,dc=example,dc=com
+            uid=user3,ou=users,dc=example,dc=com
+            uid=user4,ou=users,dc=example,dc=com
+          )
+        )
+      end
     end
 
     it 'removes extraneous spaces from DNs' do

spec/lib/gitlab/ldap/config_spec.rb

@@ -362,4 +362,38 @@ describe Gitlab::LDAP::Config do
       })
     end
   end
+
+  describe '#base' do
+    context 'when the configured base is not normalized' do
+      it 'returns the normalized base' do
+        stub_ldap_config(options: { 'base' => 'DC=example, DC= com' })
+
+        expect(config.base).to eq('dc=example,dc=com')
+      end
+    end
+
+    context 'when the configured base is normalized' do
+      it 'returns the base unaltered' do
+        stub_ldap_config(options: { 'base' => 'dc=example,dc=com' })
+
+        expect(config.base).to eq('dc=example,dc=com')
+      end
+    end
+
+    context 'when the configured base is malformed' do
+      it 'returns the base unaltered' do
+        stub_ldap_config(options: { 'base' => 'invalid,dc=example,dc=com' })
+
+        expect(config.base).to eq('invalid,dc=example,dc=com')
+      end
+    end
+
+    context 'when the configured base is blank' do
+      it 'returns the base unaltered' do
+        stub_ldap_config(options: { 'base' => '' })
+
+        expect(config.base).to eq('')
+      end
+    end
+  end
 end