Stop converting binary user attributes to text during LDAP setup (fixes #2140)

lib/ldap/user.rb

@@ -120,38 +120,34 @@ class Ldap
     #  #=> {:dn=>"dn (e. g. CN=Administrator,CN=Users,DC=domain,DC=tld)", ...}
     #
     # @return [Hash{Symbol=>String}] The available User attributes as key and the name and an example as value.
-    def attributes(filter: nil, base_dn: nil)
+    def attributes(custom_filter: nil, base_dn: nil)
-
+      @attributes ||= begin
-      filter ||= filter()
+        attributes     = {}.with_indifferent_access
-
-      attributes       = {}
-      known_attributes = BLACKLISTED.dup
-      lookup_counter   = 1
-
-      @ldap.search(filter, base: base_dn) do |entry|
-        new_attributes = entry.attribute_names - known_attributes
-
-        if new_attributes.blank?
-          lookup_counter += 1
-          # check max 50 entries with
-          # the same attributes in a row
-          break if lookup_counter == 50
-          next
-        end
-
-        new_attributes.each do |attribute|
-          value = entry[attribute]
-          next if value.blank?
-          next if value[0].blank?
-
-          example_value         = value[0].force_encoding('UTF-8').utf8_encode(fallback: :read_as_sanitized_binary)
-          attributes[attribute] = "#{attribute} (e. g. #{example_value})"
-        end
-
-        known_attributes.concat(new_attributes)
         lookup_counter = 0
+
+        # collect sample attributes
+        @ldap.search(custom_filter || filter, base: base_dn) do |entry|
+          pre_merge_count = attributes.count
+
+          attributes.reverse_merge!(entry.to_h
+                                         .except(*BLACKLISTED)
+                                         .transform_values(&:first)
+                                         .compact)
+
+          # check max 50 entries with the same attributes in a row
+          lookup_counter = (pre_merge_count < attributes.count ? 0 : lookup_counter.next)
+          break if lookup_counter >= 50
+        end
+
+        # format sample values for presentation
+        attributes.each do |name, value|
+          attributes[name] = if value.encoding == Encoding.find('ascii-8bit')
+                               "#{name} (binary data)"
+                             else
+                               "#{name} (e.g., #{value.utf8_encode})"
+                             end
+        end
       end
-      attributes
     end
 
     # The active filter of the instance. If none give on initialization an automatic lookup is performed.

spec/lib/ldap/user_spec.rb

@@ -2,6 +2,7 @@ require 'rails_helper'
 # rails autoloading issue
 require 'ldap'
 require 'ldap/user'
+require 'tcr/net/ldap'
 
 RSpec.describe Ldap::User do
 
@@ -195,4 +196,51 @@ RSpec.describe Ldap::User do
       end
     end
   end
+
+  # Each of these test cases depends on
+  # sample TCP transmission data recorded with TCR,
+  # stored in test/data/tcr_cassettes.
+  context 'on mocked LDAP connections' do
+    around do |example|
+      cassette_name = example.description.gsub(/[^0-9A-Za-z.\-]+/, '_')
+
+      begin
+        original_tcr_format = TCR.configuration.format
+        TCR.configuration.format = 'marshal'
+        TCR.use_cassette("lib/ldap/user/#{cassette_name}") { example.run }
+      ensure
+        TCR.configuration.format = original_tcr_format
+      end
+    end
+
+    describe '#attributes' do
+      let(:subject) { described_class.new(config, ldap: ldap) }
+      let(:ldap)    { Ldap.new(config) }
+      let(:config) do
+        { 'host_url'  => 'ldap://localhost',
+          'options'   => { 'dc=example,dc=org' => 'dc=example,dc=org' },
+          'option'    => 'dc=example,dc=org',
+          'base_dn'   => 'dc=example,dc=org',
+          'bind_user' => 'cn=admin,dc=example,dc=org',
+          'bind_pw'   => 'admin' }.with_indifferent_access
+      end
+
+      # see https://github.com/zammad/zammad/issues/2140
+      #
+      # This method grabs sample values of user attributes on the LDAP server.
+      # It used to coerce ALL values to Unicode strings, including binary attributes
+      # (e.g., usersmimecertificate / msexchmailboxsecuritydescriptor),
+      # which led to valid Unicode gibberish (e.g., "\u0001\u0001\u0004...")
+      #
+      # When saving these values to the database,
+      # ActiveRecord::Store would convert them to binary (ASCII-8BIT) strings,
+      # which would then break #to_json with an Encoding::UndefinedConversion error.
+      it 'skips binary attributes (#2140)' do
+        Setting.set('ldap_config', subject.attributes)
+
+        expect { Setting.get('ldap_config').to_json }
+          .not_to raise_error
+      end
+    end
+  end
 end

spec/support/tcr.rb

@@ -0,0 +1,5 @@
+TCR.configure do |config|
+  config.cassette_library_dir = 'test/data/tcr_cassettes'
+  config.hook_tcp_ports = [389] # LDAP
+  config.format = 'yaml'
+end