diff --git a/lib/ldap/user.rb b/lib/ldap/user.rb index 4b228fb27..0d39bb00a 100644 --- a/lib/ldap/user.rb +++ b/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) - - filter ||= filter() - - 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) + def attributes(custom_filter: nil, base_dn: nil) + @attributes ||= begin + attributes = {}.with_indifferent_access 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. diff --git a/spec/lib/ldap/user_spec.rb b/spec/lib/ldap/user_spec.rb index 8bf5657ec..ad567050e 100644 --- a/spec/lib/ldap/user_spec.rb +++ b/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 diff --git a/spec/support/tcr.rb b/spec/support/tcr.rb new file mode 100644 index 000000000..b744d365e --- /dev/null +++ b/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 diff --git a/test/data/tcr_cassettes/lib/ldap/user/skips_binary_attributes_2140_.marshal b/test/data/tcr_cassettes/lib/ldap/user/skips_binary_attributes_2140_.marshal new file mode 100644 index 000000000..6bcc3fd82 Binary files /dev/null and b/test/data/tcr_cassettes/lib/ldap/user/skips_binary_attributes_2140_.marshal differ