Merge branch 'sh-ldap-handle-utf8-dn' into 'master'

Convert ASCII-8BIT LDAP DNs to UTF-8 to avoid unnecessary user deletions

Issue #1159 exposed a bug where LDAP DNs would be loaded in ASCII-8BIT encoding
but compared against UTF-8-encoded values. This comparison would always
fail, causing the LDAP group sync to evict users with Unicode characters.
The problem was quietly masked because the user would be re-added later
in the group sync worker.

This commit forces the UTF-8 encoding and falls back to the original value
if that fails.

The net-ldap library has an outstanding issue
(https://github.com/ruby-ldap/ruby-net-ldap/issues/4) to load data in UTF-8
format instead of ASCII-8BIT. Per
https://tools.ietf.org/html/rfc4514#section-3, LDAP DNs should be in UTF-8.

See merge request !828

CHANGELOG-EE.md

@@ -2,7 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 ## 8.14.0 (2016-11-22)
   - gitlab:check rake task checks ES version according to requirements
-
+  - Convert ASCII-8BIT LDAP DNs to UTF-8 to avoid unnecessary user deletions
 
 ## 8.13.1 (2016-10-25)
   - Hide multiple board actions if user doesnt have permissions. !816

lib/ee/gitlab/ldap/sync/proxy.rb

@@ -76,22 +76,35 @@ module EE
                 next
               end
 
-              # If there is more than one key/value set we must have a full DN,
-              # or at least the probability is higher.
-              if parsed_dn.count > 2
-                dn
-              elsif parsed_dn[0] == 'uid'
-                dn_for_uid(parsed_dn[1])
-              else
-                logger.warn { "Found potentially malformed/incomplete DN: '#{dn}'" }
-                dn
-              end
+              final_dn =
+                # If there is more than one key/value set we must have a full DN,
+                # or at least the probability is higher.
+                if parsed_dn.count > 2
+                  dn
+                elsif parsed_dn[0] == 'uid'
+                  dn_for_uid(parsed_dn[1])
+                else
+                  logger.warn { "Found potentially malformed/incomplete DN: '#{dn}'" }
+                  dn
+                end
+
+              clean_encoding(final_dn)
             end
 
             # Remove `nil` values generated by the rescue above.
             dns.compact!
           end
 
+          # net-ldap only returns ASCII-8BIT and does not support UTF-8 out-of-the-box:
+          # https://github.com/ruby-ldap/ruby-net-ldap/issues/4
+          def clean_encoding(dn)
+            return dn unless dn.present?
+
+            dn.force_encoding('UTF-8')
+          rescue
+            dn
+          end
+
           def member_uid_to_dn(uid)
             identity = Identity.find_by(provider: provider, secondary_extern_uid: uid)
 

spec/lib/ee/gitlab/ldap/sync/proxy_spec.rb

@@ -25,6 +25,18 @@ describe EE::Gitlab::LDAP::Sync::Proxy, lib: true do
       expect(sync_proxy.dns_for_group_cn('ldap_group1')).to eq([])
     end
 
+    context 'with a valid LDAP group that contains ASCII-8BIT-encoded Unicode data' do
+      let(:username) { 'Méräy'.force_encoding('ASCII-8BIT') }
+      let(:dns) { [user_dn(username)] }
+
+      it 'return members DNs' do
+        ldap_group = ldap_group_entry(dns)
+        stub_ldap_group_find_by_cn('ldap_group1', ldap_group, adapter)
+
+        expect(sync_proxy.dns_for_group_cn('ldap_group1').first).to include("uid=Méräy")
+      end
+    end
+
     context 'with a valid LDAP group that contains members' do
       # Create some random usernames and DNs
       let(:usernames) { (1..4).map { FFaker::Internet.user_name } }